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 Setup Settings constant before the database is connected #23038

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented May 17, 2024

This reverts PR #22853 since it caused Settings to not load SettingsChanges

Before:

>> Settings.log.level 
=> "info"
>> Settings.log.level = "debug"
=> "debug"
>> Vmdb::Settings.save!(MiqServer.my_server, Settings.to_h)
>> Settings.log.level 
=> "debug"
>> 
adam@workstation:~/src/manageiq/manageiq$ rails c
>> Settings.log.level 
=> "info"
>> SettingsChange.all
=> [#<SettingsChange:0x00007f656a539bf8 id: 3, resource_type: "MiqServer", resource_id: 1, key: "/log/level", value: "debug", created_at: Wed, 15 May 2024 17:04:49.507077000 UTC +00:00, updated_at: Wed, 15 May 2024 17:04:49.507077000 UTC +00:00>]

After:

>> SettingsChange.first
=>                                                          
#<SettingsChange:0x00007fb0ff1ffad8                         
 id: 3,                                                     
 resource_type: "MiqServer",                                
 resource_id: 1,                                            
 key: "/log/level",                                         
 value: "debug",
 created_at: Wed, 15 May 2024 17:04:49.507077000 UTC +00:00,
 updated_at: Wed, 15 May 2024 17:04:49.507077000 UTC +00:00>
>> Settings.log.level
=> "debug"

…db_settings_before_database"

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

miq-bot commented May 17, 2024

Checked commit agrare@a4b57ee 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. 👍

@Fryguy Fryguy merged commit b80344a into ManageIQ:master May 17, 2024
8 checks passed
@Fryguy
Copy link
Member

Fryguy commented May 17, 2024

@jrafanie Please be aware this will re-break Rails 7, so we need to fix a different way.

@Fryguy Fryguy self-assigned this May 17, 2024
@agrare agrare deleted the revert_initialize_vmdb_settings_before_database branch May 17, 2024 14:52
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.

3 participants