-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Rails #2872
Comments
@rsgonzal @viviancan I have update version Rails and some gem. When I update version Rails have many method helpers throw error on view and testing. I need more time to finish ticket, thanks! |
Thanks for the update! Please let us know if you have any questions. |
@rsgonzal @shaunxp20 @viviancan
|
From what I could tell, we're not even using CoffeeScript, so I think it's safe to remove this. 👍 Refs: #2872
Updating this to RSpec 4.x, instead of 5.x, seems safer to make a smaller jump first; after some other kinks get worked out, then we can look at updating this to version 5.x. 👍 I also removed the Selenium web drivers, unless I'm missing something, I don't think we need those at this point. Refs: #2872
`config.ignore_localhost = true` - this seems like a valid change, but at this point there are too many moving pieces to the Rails upgrade, which I'm trying to minimize, so that's why I'm removing it `config.allow_http_connections_when_no_cassette = true` - we definitely don't want this setting enabled `github-releases.githubusercontent.com` - this seems to be the new domain where the Chrome/Gecko drivers get downloaded from, so we want to allow connections to there Refs: #2872
Just fixing some linting/formatting issues. Refs: #2872
- Explicitly expect `true` and `false`, instead of truthy/falsey values - Apparently "null" gets returned for empty survey values now 🤷 - Ensure the admin is fully signed in (by expecting the text "Welcome back!") before moving on to the next steps Refs: #2872
"Judge" is a better name than "user". “There are only two industries that call their customers ‘users’: illegal drugs and software” - Edward Tufte Refs: #2872
- Reverting the changes Agility made - Marking them as pending; spending too much time on trying to fix them, need a break from them for now Refs: #2872
- Explicitly expect `true` and `false`, instead of truthy/falsey values - Be explicit about expecting empty strings - Ensure the admin is fully signed in (by expecting the text "Welcome back!") before moving on to the next steps Refs: #2872
"Judge" is a better name than "user". “There are only two industries that call their customers ‘users’: illegal drugs and software” - Edward Tufte Refs: #2872
- Reverting the changes Agility made - Marking them as pending; spending too much time on trying to fix them, need a break from them for now Refs: #2872
Seeing if this works better. Some Stack Overflow answers for reference: https://stackoverflow.com/a/60695813 https://stackoverflow.com/a/52405269 https://stackoverflow.com/a/44916498 Refs: #2872
These specs are failing intermittently, after some researching I think this new way will work better, we'll see! Some Stack Overflow answers for reference: https://stackoverflow.com/a/60695813 https://stackoverflow.com/a/52405269 https://stackoverflow.com/a/44916498 Refs: #2872
Changing these back to their original form, i.e., removing the changes from Agility. Refs: #2872
Attempting to fix another flaky test. Refs: #2872
Attempting to fix another flaky test. Refs: #2872
This is getting to a pretty good/stable spot. I think the next step is to get it on QA for some additional testing (I only did very basic manual testing). Some random notes/comments:
|
Thank you for your help getting this one started, @tamtran-agilityio!! We appreciate it! 🙌 🎉 😀 |
I also forgot to mention, I created a new config setting for the mentor training doc I was getting some weird/random test failure about rendering this doc, so the tests won't render it, but QA and PROD will. |
Just seeing if this will fix the error on QA: Unable to load application: RuntimeError: Couldn't find Active Storage configuration in /app/config/storage.yml I found the solution here: https://stackoverflow.com/a/66179690 Refs: #2872
Running into a "stack level too deep" error: /GEM_ROOT/gems/scout_apm-2.6.10/lib/scout_apm/layer_children_set.rb:30 Updating Scout to see if that fixes it. Refs: #2872
Last week when I was trying to deploy this to QA I was getting a In the partial backtrace (below), it kept referencing New Relic and Scout. I ended up removing both New Relic and Scout in order to deploy to QA successfully. We'll probably want to add one or both of these back at some point.
|
I mentioned last week on our daily stand-up, when we deploy this to production I think we should have a deployment plan, for things like:
|
Like I mentioned in stand, I'm not getting emails ( including parent consent) |
I'm not sure new accounts are created properly? I'm looking at dataclips to try and find the ones I created last week and I don't see them. I can't remember those emails.. But Christina just created this account and it is not showing up in the admin but she can login: christina+m1@technovation.org Update: found mine roxana+mentor826@technovation.org and i believe i made roxana+826@technovation.org as well |
Apparently this uploads heap dumps to s3, according to this commit, 7d819bd: ``` Add sidekiq profiler middleware that puts periodic heap dumps on s3 ``` Anywho, it's causing an error w/ the Rails upgrade stuff, and also we aren't even using it! /attribute_accessors.rb:202:in `mattr_accessor' Sep 01 10:41:59 technovation-qa app/worker.1 /app/lib/sidekiq_profiler.rb:23:in `singleton class' Refs: #2872
`default_retries_exhausted` is no longer supported in Sidekiq, I couldn't find anything in the Sidekiq changelog about it being removed, but I looked at the latest code and it's no longer there. So, anywho, I updated our config to remove it from there. Refs: #2872
Okay, I think we finally have this one resolved. Emails (and a bunch of other things) get run/sent in background jobs. The jobs weren't running because there were a couple of issues with the job runner (Sidekiq) configuration; I removed two antiquated settings (that were added in 2017) and that seems to have fixed it. There were 96 jobs queued up before the fix and they all seem to have run, so I think we're good (at least as far as this issue is concerned). 👍 😄 |
Yay! Looks good. I logged into Checkr and don't see myself. I wonder if QA just isn't hooked up? UPDATE: I found myself, so it is sending info to checkr but i am pending UPDATE UPDATE: I did some digging and it looks like when people entered their real SSN on QA, they remained pending 🤔 |
So i signed up and used the bypass code and it worked 🤷🏻♀️ |
Just following up with this, here are the two commits with more details about the removed configuration settings: The first one was basically backing up the stacktrace logs and sending them to S3. I'm pretty sure we don't need this functionality, I have never used it. I guess if we need something like this, we can revisit it then, imho. The second one was marking jobs as "dead" if a job reached its maximum number of retries. I don't know the history behind this functionality, but Sidekiq already has a "dead" queue that can be accessed from the "Background Jobs" link in the admin. The removed code wasn't doing anything w/ the dead jobs, so afaict we're not missing any functionality by removing this code. |
I'm not sure if this behavior is expected or not on QA (I'm not really sure how Checkr is supposed to work and if it's supposed to work differently on QA). Anywho, let me know if there's something I need to look at. 😃 |
I think it's fine? We will probably want to double check once it hits production? |
Apparently this uploads heap dumps to s3, according to this commit, 7d819bd: ``` Add sidekiq profiler middleware that puts periodic heap dumps on s3 ``` Anywho, it's causing an error w/ the Rails upgrade stuff, and also we aren't even using it! /attribute_accessors.rb:202:in `mattr_accessor' Sep 01 10:41:59 technovation-qa app/worker.1 /app/lib/sidekiq_profiler.rb:23:in `singleton class' Refs: #2872
`default_retries_exhausted` is no longer supported in Sidekiq, I couldn't find anything in the Sidekiq changelog about it being removed, but I looked at the latest code and it's no longer there. So, anywho, I updated our config to remove it from there. Refs: #2872
Woohoo, we finally got this one deployed to production. Thank you everyone for your help with this! 🙌 🥳 🏅 🎊 🎈 🎉 |
Deliverables
Deployment Notes
Do not deploy
The text was updated successfully, but these errors were encountered: