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

Use rails 7 cache format as 7 can read either 6 or 7 #23210

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie force-pushed the use_rails7_cache_format_version branch from 4a2abca to 7838a74 Compare September 26, 2024 18:58
@miq-bot
Copy link
Member

miq-bot commented Sep 26, 2024

Checked commit jrafanie@7838a74 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 1 offense detected

config/application.rb

@Fryguy
Copy link
Member

Fryguy commented Sep 26, 2024

Not sure if the rubocop is important or not

@kbrock
Copy link
Member

kbrock commented Sep 26, 2024

Huh. I wonder what class is there - if it is a Version or something

Maybe ask if the version < 7.1 ?

@jrafanie
Copy link
Member Author

Huh. I wonder what class is there - if it is a Version or something

Maybe ask if the version < 7.1 ?

It's a float. It's worrying about precision in case you do math operations. This is a straight comparison with a fixed float so I think we can ignore it. Let me kick the failing sporadic auth failure.

# TODO: Remove this once we move to config.load_defaults 7.0 as this is the default.
# Note, rails 7 can read cache format from 6 or 7 so there is no risk if you're running rails 7.
# See: https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#new-activesupport-cache-serialization-format
warn "Warning: Remove redundant config.active_support.cache_format_version = 7.0 from #{__FILE__}:#{__LINE__ + 1} if using config.load_defaults 7.0" if config.active_support.cache_format_version == 7.0
Copy link
Member Author

Choose a reason for hiding this comment

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

note, this is nil if you load defaults earlier than 7.0

Copy link
Member

Choose a reason for hiding this comment

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

Will we get these warnings for a while while we support both 6 and 7?
Or are we dropping 6 super quick as we migrate to 7.2 land?

Copy link
Member Author

Choose a reason for hiding this comment

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

This warning will only occur when we're on rails 7.0+ where we're using config.load_defaults 7.0 or newer. It's a reminder to remove this setting when #23176 is merged.

@jrafanie
Copy link
Member Author

Ok, this is ready to go.

@Fryguy Fryguy merged commit 1bd9520 into ManageIQ:master Sep 30, 2024
8 checks passed
@jrafanie jrafanie deleted the use_rails7_cache_format_version branch October 3, 2024 13:28
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.

4 participants