-
Notifications
You must be signed in to change notification settings - Fork 504
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
Rails > 6 and Ruby > 3 support #577
base: master
Are you sure you want to change the base?
Conversation
…base Undo some unneeded changes and more code improvements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @thiagopradi , just so you have a headsup:
I reviewed all files, and the ones that change implementation are:
(the other ones are just linter/style/really-small-refactors)
- https://github.com/thiagopradi/octopus/pull/577/files#diff-e1d5819dd27bb595e87c091f986a1ac83373a6ea6c35672342d43f64c7f1d784
- https://github.com/thiagopradi/octopus/pull/577/files#diff-3162c2d5b3f3f191c5d77fa64a532992b39058056d9fb5d95425670e2c8b9084
- https://github.com/thiagopradi/octopus/pull/577/files#diff-d16e515b7940dd390eebea1d92b0fd166fa41beabf31a3f98acf80c6747b397f
- https://github.com/thiagopradi/octopus/pull/577/files#diff-01bb2bfb627bac9391edc80ee66deb9715fc054508d1cdfe059b91f10fae2033
if File.exist?(file_name) || File.symlink?(file_name) | ||
config ||= HashWithIndifferentAccess.new(YAML.load(ERB.new(File.read(file_name)).result))[Octopus.env] | ||
config = if File.exist?(file_name) || File.symlink?(file_name) | ||
HashWithIndifferentAccess.new(YAML.safe_load(ERB.new(File.read(file_name)).result))[Octopus.env] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to outsource it in another PR as this one isn't related to rails and would probably help to have it upstreamed faster.
Also, we should allow alisases here (or have a config for that)
We extensively rely on the
ar-octopus
gem in various projects within our company. However, due to them being on an older version of Rails that lacks the matured functionalities of the new multiple database support, we are unable to leverage it. Nevertheless, this gem remains crucial for effectively managing our shards and facilitating database replica reading.To ensure compatibility with Rails version
> 6
and Ruby> 3
, modifications were required for our application to function correctly. Unfortunately, some of these changes may break existing applications utilizing this gem. However, I think we can collaborate to review these modifications and find a way to incorporate them into the gem, making it adaptable to everyone's use cases.