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: Agena3000 integration #6594

Merged
merged 7 commits into from
Apr 15, 2022
Merged

feat: Agena3000 integration #6594

merged 7 commits into from
Apr 15, 2022

Conversation

stephanegigandet
Copy link
Contributor

@stephanegigandet stephanegigandet commented Apr 11, 2022

  • script to run the Agena3000 import (modeled on the Equadis script), to be scheduled in a crontab
  • added Agena3000 to the list of connected databases (used in org profiles admin section to enable them, and in the product pages to display the data sources)
  • also fixed a few issues related to image upload, and slightly improved the code and documentation for image upload (a lot of it can be improved, but we can do it another time)

Part of #6537

@stephanegigandet stephanegigandet added the 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers label Apr 11, 2022
@stephanegigandet stephanegigandet requested a review from a team as a code owner April 11, 2022 16:15
@github-actions github-actions bot added Data import Display Products Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org labels Apr 11, 2022
@stephanegigandet stephanegigandet changed the title Agena3000 integration feat: Agena3000 integration Apr 11, 2022
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.

LGTM, some minor comments.

lib/ProductOpener/Images.pm Outdated Show resolved Hide resolved
if ($file =~ /\.($extensions)$/i) {
$extension = lc($1) ;
}
if ($file =~ /\.($extensions)$/i) {
Copy link
Member

Choose a reason for hiding this comment

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

renaming $extensions to $supported_extensions would help improve readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will do

my $filename = get_string_id_for_lang("no_language", remote_addr(). '_' . $`);
}

my $filename = get_string_id_for_lang("no_language", remote_addr(). '_' . $`);
Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment ? I don't understand what we do here…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a comment, it's just a name for a temporary file, prefixed with the user ip address + a normalized version of the original file name uploaded by the user

$lock_path = "$product_www_root/images/products/$path/$imgid.lock";
}
my $lock_path = "$product_www_root/images/products/$path/$imgid.lock";
while ((-e $lock_path) or (-e "$product_www_root/images/products/$path/$imgid.jpg")) {
Copy link
Member

Choose a reason for hiding this comment

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

can't we first search for max imgid already present, to avoid a long check on files (in my experience it can really degrade perfs)

Copy link
Member

Choose a reason for hiding this comment

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

can't we first search for max imgid already present
with ls I mean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure we could do that


rm -rf /srv2/off-pro/agena3000-data-tmp
mkdir /srv2/off-pro/agena3000-data-tmp
find /home/sftp/agena3000/RECETTE/Fiches/ -mtime -2 -type f -exec cp {} /srv2/off-pro/agena3000-data-tmp/ \;
Copy link
Member

Choose a reason for hiding this comment

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

to be sure not to miss an export, another way is to touch a file at the end, and use find with -newer

And if the file does not yet exists fallback to 2 days.

stephanegigandet and others added 2 commits April 14, 2022 09:29
Co-authored-by: Alex Garel <alex@garel.org>
@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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@stephanegigandet stephanegigandet merged commit a6841ea into main Apr 15, 2022
@stephanegigandet stephanegigandet deleted the agena3000 branch April 15, 2022 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data import Display 🖼️ Images 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers Products Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org
Projects
Development

Successfully merging this pull request may close these issues.

2 participants