-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Redirecting MDB feeds to TDG #1524
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
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
functions-python/tasks_executor/tests/tasks/data_import/test_redirect_mdb_feeds.py
Show resolved
Hide resolved
functions-python/tasks_executor/src/tasks/data_import/transportdatagouv/update_tdg_redirects.py
Show resolved
Hide resolved
functions-python/tasks_executor/src/tasks/data_import/transportdatagouv/update_tdg_redirects.py
Show resolved
Hide resolved
functions-python/tasks_executor/tests/tasks/data_import/test_redirect_mdb_feeds.py
Show resolved
Hide resolved
qcdyx
left a comment
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
davidgamez
left a comment
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!
|
Post release we can set the TDG redirects by running the {
"task": "mdb_to_tdg_redirect",
"payload": {
"dry_run": false
}
}I suggest running the function after TDG feeds as set as |
davidgamez
left a comment
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!
| -- 2. Set default for future inserts | ||
| ALTER TABLE feed | ||
| ALTER COLUMN operational_status | ||
| SET DEFAULT 'published'; |
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.
👍
|
|
||
| feed.status = _compute_status_from_end_date(resource.get("metadata") or {}) | ||
| feed.operational_status = "wip" | ||
| feed.operational_status = "published" |
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.
👍
Summary:
This pull request introduces a new mechanism for redirecting duplicate MDB feeds to their corresponding TDG-imported feeds, along with comprehensive tests and a supporting database constraint update. The main changes include the implementation of a new handler for updating redirects based on a CSV mapping, integration of this handler into the task executor, and the addition of tests to ensure correctness. Additionally, a database migration enforces non-null operational status for feeds.
Redirects and Data Import Improvements:
update_tdg_redirects_handler) inupdate_tdg_redirects.pyto process a CSV mapping of MDB to TDG feeds, ensuring database redirects are created or updated as needed, with support for dry-run and batch commits."mdb_to_tdg_redirect"task inmain.py, making the handler available as a callable task with a description. [1] [2]Testing:
test_redirect_mdb_feeds.py) covering various scenarios for the redirect logic, including creation, missing feeds, existing redirects, and CSV load failures.Database Migration:
fix_operation_status_constraint.sql) to set all nulloperational_statusvalues in thefeedtable to'published'and enforce the column as non-null, referenced in the main changelog. [1] [2]Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything