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

Revert unsafe initializer #118

Merged

Conversation

lunks
Copy link
Contributor

@lunks lunks commented Oct 26, 2020

This is related to #113, #39 and #53.

In development, after a hot reload from webpacker, Apartment::Tenant.current reverts to public probably to the issue explained in #113, resulting in an error in our code. We haven't had the issue in production like @fsateler, but in development it is pretty much all the time after a reload.

Given this is a fork of the old apartment and the probably number one reason it exists is to support it forward (like Rails 6), I think it makes sense to keep the behaviour of the previous gem for now.

@fsateler
Copy link
Contributor

Can we please get at least the option to disable the initializer?

@lunks
Copy link
Contributor Author

lunks commented Oct 27, 2020

Can we please get at least the option to disable the initializer?

Maybe something like APARTMENT_DISABLE_INIT=true as env var?

@fsateler
Copy link
Contributor

Maybe something like APARTMENT_DISABLE_INIT=true as env var?

Sounds good to me!

@lunks lunks force-pushed the chore/revert-unsafe-initializer branch 2 times, most recently from db4938f to dd4df1a Compare October 28, 2020 18:20
@lunks lunks changed the base branch from development to master October 28, 2020 18:28
@lunks lunks force-pushed the chore/revert-unsafe-initializer branch from 2b0f436 to bbb0bc8 Compare October 28, 2020 18:42
@lunks lunks force-pushed the chore/revert-unsafe-initializer branch from bbb0bc8 to c5c7349 Compare October 28, 2020 18:43
@lunks lunks force-pushed the chore/revert-unsafe-initializer branch from c5c7349 to 426bb6c Compare October 28, 2020 19:34
@lunks
Copy link
Contributor Author

lunks commented Oct 28, 2020

Maybe something like APARTMENT_DISABLE_INIT=true as env var?

Sounds good to me!

Done.

@rpbaltazar
Copy link
Contributor

Thank you for looking at this @lunks! 👍

@rpbaltazar rpbaltazar merged commit baa9975 into rails-on-services:master Nov 9, 2020
@lunks
Copy link
Contributor Author

lunks commented Nov 9, 2020

Thank you for looking at this @lunks! 👍

Thanks for merging it in! 🎉 Nice work keeping apartment up to date, we wouldn't be able to upgrade our app to Rails 6 without this.

rpbaltazar added a commit that referenced this pull request Dec 16, 2020
Prepare Release - 2.8.0

- Resolves #98 - Trying to generate an automatic changelog - #99
- Resolves #86 - Fixes seeding errors - #87
- Resolves #66 - Uses a transaction to create a tenant - #69
- Resolves #121 - Relaxes dependencies to allow rails 6.1 - #122
- Resolves #123 - When tests run in a transaction, new tenants in tests fail to create - #124
- Reverted unsafe initializer - #118
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