-
Notifications
You must be signed in to change notification settings - Fork 128
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
[tyr] Loki: enrich ntfs with addresses/admins #4137
Conversation
8a81968
to
1fa854a
Compare
1fa854a
to
8a33041
Compare
source/tyr/tyr/binarisation.py
Outdated
"--input", | ||
filename, | ||
"--output", | ||
filename, |
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.
I am not sure if this is a good idea to replace the input ntfs with the enriched one.
Maybe write to a different location ?
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.
it works and it should not be a problem imho.
if there is a problem, filename should not be atered by the output.
if the job succeed, the ntfs is just replaced.
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.
ok I see, the ntfs is sent to fusio2ed after, we should keep the original; ok
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.
Yeah I don't know exactly what happen with the ntfs (nor when), but I know that some things can happen (sent to fusio2ed, moved to a backup directory, ... ?).
So better be safe than sorry :)
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.
fixed in the last commit.
How can I increase the code coverage ? Sonar is warning
source/tyr/tyr/binarisation.py
Outdated
res = launch_exec("enrich-ntfs-with-addresses", params, logger) | ||
if res != 0: | ||
raise ValueError("enrich-ntfs-with-addresses failed") | ||
dataset.state = "done" |
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.
Not sure when we should set the dataset.state
to done
?
SonarCloud Quality Gate failed.
|
https://navitia.atlassian.net/browse/NAV-2217?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-53940
Just before putting the NTFS to S3 for Loki, it is enriched with addresses and administrative regions calling bragi.
For that an intermediate folder
ntfs_with_addreses
is created