Shopify / shipit-engine

Deployment coordination
https://shopify.engineering/introducing-shipit
MIT License
1.42k stars 146 forks source link

Migrate to Credentials + Remove Other Deprecation Warnings #1326

Closed kartiki975 closed 9 months ago

kartiki975 commented 10 months ago

In attempts to use the recent shipit-engine release, I noticed the following exception being thrown in CI checks:

ActiveSupport::DeprecationException: DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor of `Rails.application.credentials` and will be removed in Rails 7.2. (called from block in <class:Engine> at /tmp/bundle/ruby/3.2.0/bundler/gems/shipit-engine-03a10e90a389/[redacted]) (ActiveSupport::DeprecationException)

This PR seeks to avoid this exception by migrating to credentials as instructed here: https://github.com/Shopify/ejson-rails?tab=readme-ov-file#migrating-to-credentials.

There were 3 other deprecation warnings that were being reported during unit tests. Those have been addressed as well.

kwboyd-shopify commented 10 months ago

Just to check - were we able to test these changes in our Shipit instance?

kartiki975 commented 10 months ago

@kwboyd-shopify I didn't do a tophat but the CI checks were passing when I temporarily integrated this branch's changes here: https://github.com/Shopify/shipit/pull/2589/commits/bd719ee152153205b97bdd1d47279ef7a9a7c124

kwboyd-shopify commented 10 months ago

Gotcha! I'll pull these changes and do a quick test today before approving.

kartiki975 commented 9 months ago

With a recent tophat on internal Shipit with these changes, a 500 error is preventing batched changes to start deploying. No longer looking for reviews until I fix that issue 😞

kartiki975 commented 9 months ago

Thanks for tophatting as well, @erik-shopify! In regards to indefinite soaking, I am not seeing that on my end... that may be flakiness 🤔 let me know if you see anything unusual on any of your recent tophat attempts.