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

feat: allow admins to specify a timeout for MongoDB queries #8561

Merged
merged 4 commits into from
Jun 27, 2023

Conversation

stephanegigandet
Copy link
Contributor

This PR adds a &timeout=[number of ms] parameter that can be used by admins in order to override the default timeout (currently 50s in production), in order to be able to run queries that run longer. This could be useful for instance for monitoring queries (e.g. getting stats on recognized ingredients).

Note that the proxy timeout in nginx is currently not set (except on the pro platform where we can have big uploads), and the default is 60 sec. So in order for the timeout to have a real effect, we would also need to change the nginx config to include something like:

        proxy_connect_timeout       1200;
        proxy_send_timeout          1200;
        proxy_read_timeout          1200;
        send_timeout                1200;

@stephanegigandet stephanegigandet requested a review from a team as a code owner June 14, 2023 13:05
@teolemon teolemon added MongoDB We have 2 mongodb collections: one for current products, and one for obsolete products Admin tools labels Jun 14, 2023
@github-actions github-actions bot removed the MongoDB We have 2 mongodb collections: one for current products, and one for obsolete products label Jun 14, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jun 14, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov-commenter
Copy link

Codecov Report

Merging #8561 (7e57246) into main (4b66a91) will increase coverage by 0.00%.
The diff coverage is 43.47%.

@@           Coverage Diff           @@
##             main    #8561   +/-   ##
=======================================
  Coverage   48.73%   48.74%           
=======================================
  Files         114      114           
  Lines       21397    21406    +9     
  Branches     4786     4787    +1     
=======================================
+ Hits        10428    10434    +6     
- Misses       9685     9687    +2     
- Partials     1284     1285    +1     
Impacted Files Coverage Δ
lib/ProductOpener/Display.pm 10.00% <36.84%> (+0.12%) ⬆️
lib/ProductOpener/Data.pm 57.89% <75.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@alexgarel
Copy link
Member

@stephanegigandet if you have to change nginx, you can do it in the PR (see conf folder), even if at release time you have to copy it (on off1).

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

Cool.

@@ -793,6 +793,8 @@ sub init_request ($request_ref = {}) {
if (is_admin_user($User_id)) {
$admin = 1;
}
$request_ref->{admin} = $admin;
# TODO: remove the $admin global variable, and use $request_ref->{admin} instead.
Copy link
Member

Choose a reason for hiding this comment

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

👍

Comment on lines +4404 to +4422
=head2 get_products_collection_request_parameters ($request_ref, $additional_parameters_ref = {} )

This function looks at the request object to set parameters to pass to the get_products_collection() function.

=head3 Arguments

=head4 $request_ref request object

=head4 $additional_parameters_ref

An optional reference to a hash of parameters that should be added to the parameters extracted from the request object.
This is useful in particular to request the query to be executed on the smaller products_tags category, by passing
{ tags = 1 }

=head3 Return value

A reference to a parameters object that can be passed to get_products_collection()

=cut
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I like this kind of design.

@stephanegigandet stephanegigandet merged commit 161e3e9 into main Jun 27, 2023
@stephanegigandet stephanegigandet deleted the admin-timeout branch June 27, 2023 11:39
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