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

Setup Settings constant before the database is connected #22853

Merged

Conversation

jrafanie
Copy link
Member

In rails 7, the loading order is changed, so we need to make sure we're running Vmdb Settings init before the database connection is initialized. We can do this with the correct hook in both rails 6 and 7.

In rails 7, the loading order is changed, so we need to make sure
we're running Vmdb Settings init before the database connection is
initialized. We can do this with the correct hook in both rails 6 and 7.
@miq-bot
Copy link
Member

miq-bot commented Jan 24, 2024

Checked commit jrafanie@bbfb72e with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 👍

@jrafanie jrafanie mentioned this pull request Jan 24, 2024
@Fryguy
Copy link
Member

Fryguy commented Jan 24, 2024

Do we also need to do this with the eager_load_all_the_things initializer?

@jrafanie
Copy link
Member Author

Do we also need to do this with the eager_load_all_the_things initializer?

Good question. It certainly doesn't have a timing dependency like Vmdb::Settings.init has RE: "before database connected", so it was really "run this after the last named initializer".

I'll be checking that at some point.

@Fryguy
Copy link
Member

Fryguy commented Jan 25, 2024

ok, offhand it doesn't look as necessary to move, but I wasn't sure

@Fryguy Fryguy merged commit 1986ef3 into ManageIQ:master Jan 25, 2024
6 checks passed
@jrafanie jrafanie deleted the initialize_vmdb_settings_before_database branch January 25, 2024 15:11
agrare added a commit to agrare/manageiq that referenced this pull request May 17, 2024
…db_settings_before_database"

This reverts commit 1986ef3, reversing
changes made to b039573.
@agrare
Copy link
Member

agrare commented May 17, 2024

Opened #23038 to resolve the issues with Settings not being sourced from the database

jrafanie added a commit to jrafanie/manageiq that referenced this pull request May 20, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request May 29, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 3, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 5, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 19, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 21, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 11, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 12, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 18, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 18, 2024
This is an attempt to reimplement ManageIQ#22853 since that was
reverted in ManageIQ#23038.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 18, 2024
This is an attempt to reimplement ManageIQ#22853 since that was reverted in ManageIQ#23038.

High level: We have plugins that expect the `Settings` constant to be defined.
These will have to use the defaults from the filesystem as we don't have access to the database this early.

We can then use a rails 7 friendly location to start autoloading, the after_initialize.

Using the test from ManageIQ#23038, I was able to verify this on the rails7 branch and on master with rails 6.1:

```
joerafaniello@Joes-MBP manageiq % bin/rails c
Loading development environment (Rails 6.1.7.8)
irb(main):001:0> SettingsChange.first
=> nil
irb(main):002:0>  Settings.log.level
=> "info"
irb(main):003:0>  Settings.log.level = "debug"
=> "debug"
irb(main):004:0> Vmdb::Settings.save!(MiqServer.my_server, Settings.to_h)
=> []
irb(main):005:0>  Settings.log.level
=> "debug"
irb(main):006:0> SettingsChange.first
=> #<SettingsChange:0x00000001183e5648 id: 99000000000005, resource_type: "MiqServer", resource_id: 99000000000002, key: "/log/level", value: "debug", created_at: Thu, 18 Jul 2024 19:15:50.513221000 UTC +00:00, updated_at: Thu, 18 Jul 2024 19:15:50.513221000 UTC +00:00>
irb(main):007:0> exit
joerafaniello@Joes-MBP manageiq % bin/rails c
Loading development environment (Rails 6.1.7.8)
irb(main):001:0>  Settings.log.level
=> "debug"
```
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jul 18, 2024
This is an attempt to reimplement ManageIQ#22853 since that was reverted in ManageIQ#23038.

High level: We have plugins that expect the `Settings` constant to be defined.
These will have to use the defaults from the filesystem as we don't have access to the database this early.

We can then use a rails 7 friendly location to start autoloading, the after_initialize.

Using the test from ManageIQ#23038, I was able to verify this on the rails7 branch and on master with rails 6.1:

```
joerafaniello@Joes-MBP manageiq % bin/rails c
Loading development environment (Rails 6.1.7.8)
irb(main):001:0> SettingsChange.first
=> nil
irb(main):002:0>  Settings.log.level
=> "info"
irb(main):003:0>  Settings.log.level = "debug"
=> "debug"
irb(main):004:0> Vmdb::Settings.save!(MiqServer.my_server, Settings.to_h)
=> []
irb(main):005:0>  Settings.log.level
=> "debug"
irb(main):006:0> SettingsChange.first
=> #<SettingsChange:0x00000001183e5648 id: 99000000000005, resource_type: "MiqServer", resource_id: 99000000000002, key: "/log/level", value: "debug", created_at: Thu, 18 Jul 2024 19:15:50.513221000 UTC +00:00, updated_at: Thu, 18 Jul 2024 19:15:50.513221000 UTC +00:00>
irb(main):007:0> exit
joerafaniello@Joes-MBP manageiq % bin/rails c
Loading development environment (Rails 6.1.7.8)
irb(main):001:0>  Settings.log.level
=> "debug"
```
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