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

Move configuration methods to class methods #16164

Merged

Conversation

eduardoj
Copy link
Member

@eduardoj eduardoj commented May 21, 2024

We don't need to retrieve the values stored in the singleton record of the table configurations in the database for none of the ldap_enabled?, proxy_auth_mode_enabled? and amqp_namespace methods.

Moving each method to the class methods section prevents from performing unneeded database queries.

For reviewers

Before

Taking the home:Admin project page as an example, browsing http://localhost:3000/project/show/home:Admin and taking a look at the frontend log resulted in:

  • 3 direct SQL queries to the database:
    Configuration Load (0.5ms)  SELECT `configurations`.* FROM `configurations` ORDER BY `configurations`.`id` ASC LIMIT 1
    
  • 10 cache hits:
    CACHE Configuration Load (0.1ms)  SELECT `configurations`.* FROM `configurations` ORDER BY `configurations`.`id` ASC LIMIT 1
    

After

Browsing http://localhost:3000/project/show/home:Admin resulted in:

  • 3 direct SQL queries to the database:
    Configuration Load (0.5ms)  SELECT `configurations`.* FROM `configurations` ORDER BY `configurations`.`id` ASC LIMIT 1
    
  • 7 cache hits (3 less than before):
    CACHE Configuration Load (0.1ms)  SELECT `configurations`.* FROM `configurations` ORDER BY `configurations`.`id` ASC LIMIT 1
    

@eduardoj eduardoj added the review-app Apply this label if you want a review app started label May 21, 2024
@github-actions github-actions bot added the Frontend Things related to the OBS RoR app label May 21, 2024
@obs-bot
Copy link
Collaborator

obs-bot commented May 21, 2024

@eduardoj eduardoj force-pushed the refactoring/configuration_class_methods branch 3 times, most recently from f61a687 to b515569 Compare May 21, 2024 11:10
@eduardoj eduardoj marked this pull request as ready for review May 21, 2024 11:32
@eduardoj eduardoj force-pushed the refactoring/configuration_class_methods branch 2 times, most recently from f483606 to e18ccba Compare May 21, 2024 14:36
We don't need to retrieve the values stored in the singleton record of
the table `configurations` in the database for none of the
`ldap_enabled?`, `proxy_auth_mode_enabled?` and `amqp_namespace`
methods.

Moving each method to the class methods section prevents from
performing unneeded database queries.
Before, some methods were defined in the Configuration ActiveRecord
instance section. Later, those methods were moved to the class section.

Adapt the calls to those moved methods.
@eduardoj eduardoj force-pushed the refactoring/configuration_class_methods branch from e18ccba to 018a293 Compare May 21, 2024 17:11
@eduardoj
Copy link
Member Author

@hennevogel, I modified the code. Please tak a look again.

@hennevogel hennevogel merged commit 1abe6b4 into openSUSE:master May 22, 2024
20 of 21 checks passed
@eduardoj eduardoj deleted the refactoring/configuration_class_methods branch May 22, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app review-app Apply this label if you want a review app started
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants