-
Notifications
You must be signed in to change notification settings - Fork 4
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: transitFeedSyncProcessing implementation #819
Conversation
This commit: - Implements feed sync processing for Pub/Sub messages - Ensures database consistency during sync operations - Adds configuration files for feed sync settings - Includes comprehensive test coverage - Documents sync process and configuration options
functions-python/feed_sync_process_transitland/requirements.txt
Outdated
Show resolved
Hide resolved
Replaced raw SQL queries with SQLAlchemy ORM models for handling database operations in feed processing. Enhanced test coverage and updated mock configurations to align with the new ORM-based approach.
logger.error(error_msg) | ||
if "payload" in locals(): | ||
self.session.rollback() | ||
logger.debug("Database transaction rolled back due to error") |
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.
We should change this to an error level logging as it is critical
New feeds and feed updates should have |
Co-authored-by: cka-y <[email protected]>
Replaced custom logger setup with unified Logger class. Improved error handling and rollback in database transactions. Added location support and refined feed ID management. Updated test cases to reflect these changes.
Replaced direct logger calls with a unified log_message function to support both local and GCP logging. Refactored the test cases to mock enhanced logging and implemented new test scenarios to cover additional edge cases, ensuring robustness in feed processing.
…d_feed_sync_process
Refactored test coverage for feed processing, publish to batch topic, and event processing scenarios.
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 💪
Integration tests seem to be failing intermittently (not related to the changes in this PR)
Summary:
The pull request addresses feed sync processing, ensuring proper handling and consistency of feed data using Pub/Sub messages. It includes necessary configuration files, comprehensive tests, and documentation.
Key implementations include:
Added support for:
This pull request addresses the functionality described in issue https://github.com/MobilityData/product-tasks/issues/102, which is part of the https://github.com/MobilityData/product-tasks/issues/95 epic.
Expected behavior:
Feed Processing Flow:
Testing tips:
Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.sh
to make sure you didn't break anything