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

Remove initializer that was eager loading the schema cache dump #51034

Merged
merged 7 commits into from Feb 14, 2024

Conversation

rafaelfranca
Copy link
Member

This will be eager loaded by the define_attribute_methods initializer now that the schema cache can be automatically loaded for all connection if the file is present on disk after #48716.

Configure the `:schema_cache_path` in the database configuration instead.
MSG

nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this return ENV["SCHEMA_CACHE"]? Deprecated doesn't mean it should have no effect. No?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it return something, it would not have any effect. The loading of the schema cache doesn't use this environment variable, so unfortunately we will need to make a breaking change. I could raise, but if I return nil things would just work. In a different file path, but still work.

Only when schema cache is present and check_schema_cache_dump_version
is false.
This allow us to be explicit about the environment we want to load.
This will be eager loaded by the define_attribute_methods initializer
now that the schema cache can be automatically loaded for all
connection if the file is present on disk after #48716.
The message was partially duplicated because
`deprecation_warning` was being called instead of `warn`
…he databse configuration

The config in the yaml allows for more complex configuration.
@rafaelfranca rafaelfranca merged commit a9a359a into main Feb 14, 2024
7 checks passed
@rafaelfranca rafaelfranca deleted the rm-schema-cache-loading branch February 14, 2024 22:16
@agrobbin
Copy link
Contributor

Thanks for the quick update here @rafaelfranca!! Do you anticipate back-porting this to 7-1-stable, or is this considered a backwards incompatible change that won't be released until 7.2 / 8.0?

@rafaelfranca
Copy link
Member Author

It should not be necessary to backport. This is mostly a performance improvement, because the code to load schema cache should just work in 7.1

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

Successfully merging this pull request may close these issues.

None yet

3 participants