Skip to content
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

Address soft deprecation of ActiveRecord::Base.connection #869

Merged

Conversation

joshuay03
Copy link
Contributor

@joshuay03 joshuay03 commented Jun 12, 2024

Related: #705

Refs:

This will be soft deprecated in Rails 7.2.

The reason for this change is twofold.

  1. We're running on Rails Edge with active_record.permanent_connection_checkout = :deprecated and this call to #connection was logged as a violation. Our goal is to configure this with :disallowed some day. Even if this needed to be a permanent checkout, connection should ideally be swapped with lease_connection (this would require a specific check to ensure the Rails dependency is >= 7.2).

  2. Despite being called inside a with_connection block, this call actually leases a connection and doesn't return it back to the pool until the end of the request, which seems to have been intention behind ActiveRecord adapter: wrap all reads/writes in with_connection #705. This ensures we use the connection temporarily leased by with_connection.

@joshuay03 joshuay03 force-pushed the address-connection-deprecation branch from 28a27d9 to e5e221d Compare June 12, 2024 02:03
@joshuay03 joshuay03 changed the title Address soft deprecation of ActiveRecord::Base.connection Address soft deprecation of ActiveRecord::Base.connection Jun 12, 2024
@joshuay03 joshuay03 force-pushed the address-connection-deprecation branch from e5e221d to b33811b Compare June 12, 2024 02:04
@jnunemaker jnunemaker merged commit bf6a13f into flippercloud:main Aug 27, 2024
36 checks passed
@jnunemaker
Copy link
Collaborator

This is awesome. Thanks for caring and submitting this! Sorry again for the delay in merging. It's been wild over here. I'll be better in the future. 😄

@jnunemaker
Copy link
Collaborator

If you don't mind, I would love to know what you are using flipper for as well and how it is working out for you (what went well, what was hard other than what you mentioned here).

If you aren't comfortable leaving that information here in a public comment, feel free to email me directly john@fewerandfaster.com. I'm just always curious. :)

@joshuay03 joshuay03 deleted the address-connection-deprecation branch August 28, 2024 01:58
@joshuay03
Copy link
Contributor Author

If you don't mind, I would love to know what you are using flipper for as well and how it is working out for you (what went well, what was hard other than what you mentioned here).

If you aren't comfortable leaving that information here in a public comment, feel free to email me directly john@fewerandfaster.com. I'm just always curious. :)

I opened this when I was working @TandaHQ. At the time we were trialing Flipper for incrementally rolling out new features and large page rewrites in Hotwire based on region and organisation user count. However, I wasn't very involved with product work and I'm not sure if/how it's currently being utilised. I'm sure either @ghiculescu or @AwolDes would be happy to give you some feedback.

@AwolDes
Copy link

AwolDes commented Aug 29, 2024

Thanks for the tag @joshuay03!

@jnunemaker I implemented Flipper a few months ago in our product. The goal was to leverage the groups feature to release features to cohorts like organisations with 100-250 staff, internal accounts, that kind of thing to reduce the blast radius of some of the larger releases we're doing. Flipper is wrapped by a class so we can use deadlines which then raise in test and development envs.

CleanShot 2024-08-30 at 08 24 05

If you're keen to talk more I'll shoot you an email.

@jnunemaker
Copy link
Collaborator

@AwolDes always keen to talk more! We've actually been thinking of adding something like deadlines to flipper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants