closes AE-494
[skip-stages=Flakey]
[skip-crystalball]
several things going on here. in general, many of our monkeypatches
have been implemented in upstream, but in a vastly different way, so
we need to enable/configure those things
* `servers` is no longer accepted as a backwards compatible
configuration option; use `url`
* `database` is no longer accepted as a backwards compatible
configuration option; you _can_ use `db`, but preferable to
just use `url`
* no longer merge together redis.yml and cache_store.yml; if one
references the other, simply use the config from the other
* `nil_store` is no longer accepted as a backwards compatible
configuration option; use `null_store` (almost no one should
be explicitly using this anyway, so nbd)
* automatically not-even-trying when redis has previously failed
is now handled by redis-client's circuit breaker. be sure to
configure it in redis.yml/cache_store.yml/dynamic settings
* ignoring redis failures completely is already done by
RedisCacheStore; just rely on that, except for the few cases
where we use redis directly. some of these now take advantage
of a new `failsafe` kwarg (and often in combination with
pipelining), and some just handle it directly
* move logging to a RedisClient middleware
* move Twemproxy disallowed commands to a RedisClient middleware
* simplify Canvas.lookup_cache_store to have far less special casing
(in particular, patching is done automatically now)
* add ability to use Redis::Cluster (configure with `nodes` instead
of `url`)
* still override Redis::Distributed's HashRing, so that we don't
change our ring layout with the new MD5 hashing for servers. but
we got to vastly simplify the new class, due to upstream
refactorings allowing us to simply override a single method rather
than having to re-implement the entire class
* statsd reporting of redis errors is now simply passed as a callback
to RedisCacheStore, breaking CanvasCache's dependency on InstStatsd
Change-Id: I787672677a21994d40ae304dbac0fbf3a960a779
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325641
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
Fixes the Multiple Grading Periods report so that it includes students
that do not have a computed final score in a course.
closes EVAL-3531
refs EVAL-3464
refs EVAL-3502
flag=none
Test Plan:
- specs pass
Change-Id: Ib1f4aff87ae4438ccb9d5910e05fa08d66bfb644
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/328472
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Derek Williams <derek.williams@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
Product-Review: Ravi Koll <ravi.koll@instructure.com>
It's the new default debugger in ruby 3.1. Rails switched to it in 7.0,
avoids issues with Zeitwerk, has a more modern interface based on
current IRB, supports Unix Domain Sockets for remote debugging,
promises even better future maintenance due to being part of Ruby,
etc.
Change-Id: Ieaa7872f1c0308b16ae180fdb16df5dd6caa87a8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/328241
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Change-Id: Id862e4f2e9050ee8d85d99670521f8d4d9a89747
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/328158
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Isaac Moore <isaac.moore@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
well, somewhat. it's vendored for now, bugfixes and improvements
have been going into that gem, and we'd like those fixes in
Canvas.
Change-Id: Ib4f30926acddb364779b9f91b1ee129ba6b17ff0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327463
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
In newer postgres versions, this migration will fail, because of the
foreign key of `root_account_id` -> `id`. For this reason, we will
defer constraint checking when making this insert.
fixes AE-474
flag=none
test plan:
- with the fix:
- the migration completes successfully on newer postgres
- without the fix:
- the migration does not complete successfully on newer postgres
Change-Id: I1ea7b874aea4d3cae979248edafd249675fb4221
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326597
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
and apply Rails/SelectMap and Rails/RedundantActiveRecordAllMethod
the offenses in User were manually fixed to maintain sharding
correctness
[skip-stages=Flakey]
[skip-crystalball]
Change-Id: I96f877ee8474655bd62a149f3aa54b312d38a5e4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327334
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
[skip-crystalball]
and apply updated copys (RSpec/Eq and RSpec/MetadataStyle, and one
instance that the split RSpec/SpecFilePathSuffix caught)
Change-Id: I3872458f35b791f1ce3f8108a2aaf4fff2cfd612
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327204
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
not that we really need to change, but it serves as an example for future
migrations
Change-Id: Ie1a1975311c81b0c145c17af46e57e604a01eca5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327026
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
Migration-Review: Cody Cutrer <cody@instructure.com>
make it match the mismatch message for other gems more closely
Change-Id: Iab0a4089eb7862bc24504041389f10345eecacbb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326813
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
fixes CANVAS-KDBB
since the json Faraday middleware will parse them into a Hash,
instead of a string
Change-Id: I80dddcab4e4bc5fa5e3d726954bc85a4874de598
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326962
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
to deal with :info offenses
Change-Id: I6dacd48079f15dd0bcfc5c37acd9097f52c105ad
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326818
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
g/325983 requires `add_replica_identity` to be in a separate
migration from the commit that creates the table, for
complicated DBA reasons. change the phrasing of the
Migration/RootAccount cop so it doesn't contradict the
newer Migration/SetReplicaIdentityInSeparateTransaction
test plan:
- make a new migration that creates a table
and does not do `add_replica_identity`
- the cop should tell you to do that in a different migration
and not after the `create_table` block in this one
flag=none
refs AE-443
Change-Id: Id37914399d40739e9fd8a72b90745c29d9e7cbda
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326791
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This does add an additional query every time we insert a versioned record, but it
is a very lightweight query that shouldn't hit the disk and record insertions are
orders of magnitude more expensive, so it should have an insignficant performance
impact.
fixes AE-445
Change-Id: I9ae43121a221442fc1db9f1c1486778b58803917
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326670
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
also, don't set BUNDLER_VERSION env var in Docker, so that
bundler's auto-install-and-restart-with-the-correct-version
feature can work
Change-Id: I8e3722197fb3598b5c40679d997f19b3b3957ea8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326580
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
only require the exact rails gems that are needed, instead of the
whole enchilada. this slightly speeds up running individual tests,
but massively reduces lockfile churn when we update gems
Change-Id: I6c360ed03d41e02563a460e669b2cee7ee7e8cca
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/326235
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
closes VICE-3643
flag=none
test plan:
- Specs pass.
- There are plenty of tests testing sending notifications,
we can trust in them to make sure everything is working
correctly.
- To verify that this fix removes the N+1, the best way to do it
is to run manually in the rails console the queries that this
would normally generate and check that there is no N+1.
To do that, do a:
u = User.eager_load(:active_pseudonyms).take
That should do a query to the user table and LEFT OUTER JOIN
with pseudonyms preemptively.
Then, you can do:
u.suspended?
That shouldn't generate a query. If it does, the N+1 is
still there.
qa risk: low
Change-Id: I89ec1ba0a7b7443ccca995445da56351b407bda1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325730
Reviewed-by: Aaron Suggs <aaron.suggs@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
google-api-client is deprecated and unmaintained; need to use
the service specific gem. this gets us on the latest APIs, and
importantly unblocks us on updating other common dependencies
(most notably a step towards updating faraday, but also retriable)
Change-Id: I646da7dc68b8c5f6068142608c19771dafbef127
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325392
QA-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
it was disabled a year ago due to a broken API. the Google Drive LTI
is the way to do this now. this commit removes now unused related code
from the view, controllers, and the google_drive library
Change-Id: Ieccef46036c847f27e98dc8297da10d04b6721f9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325750
Tested-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
closes OUT-5824
flag=outcome_service_results_to_canvas
This PS is for hackweek Q3 2023.
Test Plan:
- Run the outcome results report and student competency report from the
root account
- Observe there are no errors and that the learning outcome group
title and learning outcome group id columns are included
- Repeat steps for a sub-account
Change-Id: If8fecdfae1f2c7b45ac803c642711a66c4b04b93
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/323738
QA-Review: Martin Yosifov <martin.yosifov@instructure.com>
Product-Review: Kyle Rosenbaum <krosenbaum@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Dave Wenzlick <david.wenzlick@instructure.com>
refs AE-57
flag=none
test plan:
- connecting with an access key still works
- connecting with a role (if present) works too
Change-Id: Id04406d85a657c071cabe0dc37e2df642a726bec
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325723
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>
this doesn't happen _frequently_ but it happens often enough
with long-running queries that it annoys admins
test plan:
* specs pass
flag=none
refs FOO-3720
Change-Id: If6e66235d83208d801a95bf9b62b339cce466508
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325661
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jason Perry <jason.perry@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
if the secondary lockfile already exists, but a pinned version
changed, it may through a slightly different error
Change-Id: I998fbf3225d8acf59ff33023ef778a8861c679c4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325094
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
no new cops, but a few fixes to existing cops (notably
Style/RedundantReturn)
Change-Id: I64a744c3d81e25329c1612e4622ca96783b0801d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/324942
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
fixes AE-377
* use a basic timeout of 1 second in canvas_rails_switcher.rb
* fix configuration of timeouts in dynamic settings (it's just a
single timeout, not a separate read and write timeout)
* avoid a circular boot problem when consul fails at boot before
Canvas::Errors is loaded
* actually avoid trying to contact Consul at all when the circuit
breaker has tripped
* reposition consul fail safes - so that they'll trip the circuit
breaker if they have to be used
Change-Id: I971beaf0a9982f3f18390b558e71b4b3bc230b16
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/324688
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Cody Cutrer