Skip to content

Changed latest_version to staging_version#1953

Open
HarishC727 wants to merge 1 commit intodatacommonsorg:masterfrom
HarishC727:run_import_changes
Open

Changed latest_version to staging_version#1953
HarishC727 wants to merge 1 commit intodatacommonsorg:masterfrom
HarishC727:run_import_changes

Conversation

@HarishC727
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the run_import.sh script to look for staging_version.txt instead of latest_version.txt when retrieving import versions from GCS. A critical issue was identified regarding inconsistency: the Python executor and various log messages still reference latest_version.txt, which will likely cause the script to fail or become out of sync with the rest of the system.

function get_latest_gcs_import_output {
echo_log "Looking for import files on GCS at $GCS_BUCKET/$IMPORT_DIR/$IMPORT_NAME..."
LATEST_VERSION=$(gsutil cat gs://$GCS_BUCKET/$IMPORT_DIR/$IMPORT_NAME/latest_version.txt)
LATEST_VERSION=$(gsutil cat gs://$GCS_BUCKET/$IMPORT_DIR/$IMPORT_NAME/staging_version.txt)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

This change introduces an inconsistency with the rest of the system. The Python executor's configuration in import-automation/executor/app/configs.py (line 77) still defines storage_version_filename as latest_version.txt. Since the Python code is responsible for writing this file, this script will look for a file (staging_version.txt) that likely won't exist or will be out of sync.

Furthermore, the log message on line 259 and the help text on line 72 still refer to latest_version.txt. If the intention is to rename the version file, it must be updated consistently across the codebase.

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.

1 participant