Skip to content

Conversation

@cka-y
Copy link
Contributor

@cka-y cka-y commented Nov 25, 2025

Summary:
Relates to #1463
This pull request introduces improvements to how the operational status and external IDs are managed for transitfeeds in the data import process, and ensures database consistency for existing feeds. The main changes are focused on updating feed metadata and maintaining accurate external ID associations.

Feed metadata management:

  • In sync_transitfeeds.py, the code now sets the operational_status of each feed to "published" during processing, ensuring all imported feeds reflect their correct status.
  • The logic for associating external IDs has been updated to check if a transitfeeds external ID already exists for a feed. If it does, a debug message is logged; if not, the external ID is added to the feed.

Database consistency:

  • A new SQL migration (feat_1249-3.sql) updates all feeds with a stable_id starting with tfs- to set their operational_status to "published", ensuring existing records are consistent with the new import logic.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with ./scripts/api-tests.sh to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

@cka-y cka-y marked this pull request as ready for review November 26, 2025 14:38
@cka-y cka-y requested a review from Copilot November 26, 2025 14:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request aims to improve the management of operational status and external IDs for transitfeeds data imports, along with a database migration to ensure consistency for existing feeds. However, there are several critical issues that need to be addressed before merging.

Key Changes

  • Sets operational_status to "published" for all transitfeeds imports in the Python import script
  • Adds logic to check for existing transitfeeds external IDs before adding new ones
  • Includes SQL migration to update existing tfs-prefixed feeds to published status

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
liquibase/changes/feat_1249-3.sql SQL migration to set operational_status for transitfeeds feeds - contains potential pattern matching issue
functions-python/tasks_executor/src/tasks/data_import/transitfeeds/sync_transitfeeds.py Updates external ID and operational_status logic - contains critical bug that replaces all external IDs

Critical Issues Found:

  1. External ID Bug (Line 110-114): The code replaces ALL existing external IDs instead of appending the transitfeeds external ID, which will cause data loss if feeds have external IDs from other sources (transitland, jbda, etc.)
  2. SQL Migration Pattern (Line 4): The WHERE clause uses 'tfs-%' pattern, but all evidence suggests transitfeeds feeds use 'mdb-%' prefix, which means the migration may not update any records
  3. Missing Test Coverage: The new operational_status behavior and external ID preservation logic are not covered by existing tests

@cka-y cka-y merged commit a0f0896 into main Nov 26, 2025
2 of 3 checks passed
@cka-y cka-y deleted the hotfix/tfs-external-id branch November 26, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants