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.

Sign up for GitHub

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

Jump to bottom

feat: add events API requests #677

Merged
merged 4 commits into from
Apr 14, 2022
Merged

feat: add events API requests #677

merged 4 commits into from
Apr 14, 2022

Conversation

alexgarel
Copy link
Member

see /~https://github.com/openfoodfacts/openfoodfacts-events

Replaces #674

What

Call openfoodfacts-events each time a user annotate an item

@alexgarel alexgarel requested a review from a team as a code owner March 30, 2022 08:51
@codecov
Copy link

codecov bot commented Mar 30, 2022 •
edited
Loading

Codecov Report

Merging #677 (8883df1) into master (1e63ca2) will increase coverage by 6.19%.
The diff coverage is 68.73%.

@@            Coverage Diff             @@
##           master     #677      +/-   ##
==========================================
+ Coverage   44.73%   50.93%   +6.19%     
==========================================
  Files          96       92       -4     
  Lines        6981     6764     -217     
==========================================
+ Hits         3123     3445     +322     
+ Misses       3858     3319     -539
Impacted Files Coverage Δ
robotoff/cli/batch.py 0.00% <ø> (ø)
robotoff/cli/insights.py 0.00% <0.00%> (ø)
robotoff/insights/dataclass.py 100.00% <ø> (ø)
robotoff/prediction/ocr/brand.py 68.62% <0.00%> (ø)
robotoff/prediction/ocr/expiration_date.py 25.71% <ø> (ø)
robotoff/prediction/ocr/label.py 72.30% <0.00%> (ø)
robotoff/prediction/ocr/product_weight.py 49.10% <0.00%> (+1.28%) ⬆️
robotoff/products.py 41.51% <ø> (+1.02%) ⬆️
robotoff/workers/listener.py 0.00% <0.00%> (ø)
robotoff/metrics.py 23.61% <10.00%> (-2.20%) ⬇️
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7e19f9...8883df1. Read the comment docs.

@alexgarel
refactor: events api requests
a6c39b0 
- no posting by default
- only use one process for async posting (perfs)
- add barcode
@alexgarel
Copy link
Member Author

Hi @ocervell I built upon your commit, but I changed it a bit.

  • added a test
  • only launch one process to have async posting (I think it was a bit too much to have one process for each events, also because we have in-process cache, which can make the fork costy after a while)
  • in dev setup: no post to events (I think few dev will have an events project setup)
  • changed location of event sending to be able to have the barcode

I would be glad if you want to review it.

@alexgarel alexgarel mentioned this pull request Mar 30, 2022
Closed
@alexgarel
fix: fix click dependency in autoblack
4b7e15c 
Because higher versions fails (may be they dropped python3.7)
@alexgarel alexgarel requested review from ocervell and teolemon March 30, 2022 15:50
ocervell
ocervell approved these changes Apr 12, 2022
View reviewed changes
Copy link
Collaborator

@ocervell ocervell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, but still needs testing within Docker local env and in pre-prod env !

robotoff/app/api.py Outdated Show resolved Hide resolved
@alexgarel
Copy link
Member Author

Thanks for the review @ocervell

I've done some test in local and it worked like a charm.

I will make a push to preprod to verify (through a specific PR on a deploy-* branch)

@alexgarel alexgarel temporarily deployed to robotoff-ml April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-ml April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-ml April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-net April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-net April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-ml April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-net April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-net April 14, 2022 14:32 Inactive
@alexgarel alexgarel temporarily deployed to robotoff-ml April 14, 2022 14:32 Inactive
@alexgarel alexgarel mentioned this pull request Apr 14, 2022
Closed
@sonarqubecloud
Copy link

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 86 Code Smells

0.0% 0.0% Coverage
0.1% 0.1% Duplication

@sonarqubecloud
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud.

@alexgarel
Copy link
Member Author

Tested ok in preprod (#718)

@alexgarel alexgarel merged commit 1f212fd into master Apr 14, 2022
@alexgarel alexgarel deleted the feat-events branch April 14, 2022 15:16
@alexgarel alexgarel mentioned this pull request Apr 14, 2022
Closed
@alexgarel alexgarel mentioned this pull request May 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ocervell ocervell ocervell approved these changes

@teolemon teolemon Awaiting requested review from teolemon

Assignees
No one assigned
Labels
GitHub Actions
Projects
🤖 Artificial Intelligence @ Open Food Facts
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexgarel @ocervell