-
-
Notifications
You must be signed in to change notification settings - Fork 384
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: Use openfoodfacts-query service for facet queries instead of product_tags collection #8947
Conversation
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.
LGTM.
Kudos @john-gom (for the off-query part firstly !).
lib/ProductOpener/Display.pm
Outdated
|
||
$count_results = [$count_results->all]->[0]; | ||
if ((not $@) and (defined $count_results)) { |
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.
there again, I think it's wiser to save $@ just after the eval block, to avoid a future mistake.
scripts/refresh_postgres.pl
Outdated
my $path = "$query_url/importfrommongo?from=$from"; | ||
print STDERR "Importing to $path\n"; | ||
my $ua = LWP::UserAgent->new(); | ||
$ua->get($path); |
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.
Is this query returning quickly ? (maybe it should)
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.
This does wait until the refresh has been completed before returning at the moment. It will be quick if it is an incremental update, but takes about 1.5 hours to do a full load.
http://world.openfoodfacts.localhost/country/united-kingdom/misc does work though |
Hi @stephanegigandet , just to clarify, you should only need this kind of configuration if something running inside docker needs to access a service running on the host. So this would either be the query service running in docker trying to access MongoDB running on the host (not in Docker) or Product Opener running in Docker trying to access the Query service running on the host (not in Docker). If the Query service is running in Docker and is accessing MongoDB running in docker then it should be able to access the MongoDB database using the "mongodb" host name as the Query service joins the po_default network. What scenario were you trying? |
Hi @stephanegigandet . I hadn't realised that we supported multiple filters like this. I can enhance the query handling to support this relatively easily |
Hi @stephanegigandet , this should be fixed now |
Codecov Report
@@ Coverage Diff @@
## main #8947 +/- ##
==========================================
- Coverage 47.95% 47.93% -0.03%
==========================================
Files 64 64
Lines 20013 20052 +39
Branches 4850 4868 +18
==========================================
+ Hits 9597 9611 +14
- Misses 9168 9186 +18
- Partials 1248 1255 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
I was following the instructions on https://github.com/openfoodfacts/openfoodfacts-query#prepare-for-development |
Hi @stephanegigandet , @raphael0202 , @alexgarel , I have reverted the code so that the products_tags collection is used if the QUERY_URL is not available (or the call to the query service fails). Would you be happy to take a look at the latest changes to check you have no concerns. Also, the tests are not passing but I can't figure out why my changes should have affected this. If anyone can help I would be vary grateful. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
You are getting a 404 because the returned product count is 0 I think. |
Thanks. Figured it out, the code was still going to product_tags when no_cache=1 |
What
Use the Postgres-based query service for counts and aggregations. Updated the product_tags refresh scripts to refresh this database instead
Related issue(s) and discussion