Skip to content

Refactor weather-sp for adding recursive copy files#552

Open
shail-parekh wants to merge 6 commits into
google:mainfrom
shail-parekh:modify-weather-sp-for-copy-files
Open

Refactor weather-sp for adding recursive copy files#552
shail-parekh wants to merge 6 commits into
google:mainfrom
shail-parekh:modify-weather-sp-for-copy-files

Conversation

@shail-parekh

Copy link
Copy Markdown
Contributor

Refactor copy_dir for robust GCS uploads

  • Optimize GCS transfers using bulk gcloud storage cp -r with explicit file lists.

Comment thread weather_sp/splitter_pipeline/file_splitters.py Outdated
Comment thread weather_sp/splitter_pipeline/file_splitters.py Outdated
Comment thread weather_sp/splitter_pipeline/file_splitters.py Outdated
Comment thread weather_sp/splitter_pipeline/file_splitters.py Outdated
Comment thread weather_sp/splitter_pipeline/file_splitters.py Outdated
copy_dir(tmpdir, prefix)
self.logger.info('Finished uploading %r', self.input_path)
else:
raise RuntimeError(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it advisable to raise a RuntimeError for this? As this does not specifically mean it's a pipeline issue/bug.

Thoughts @Piyush-Ingale @mahrsee1997 ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, we could raise a ValueError. Alternatively, we could log it as an error to prevent the entire pipeline from crashing if a failure occurs while generating split files for only one of the input files.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed then let's log it as an error or a warning

@shail-parekh
shail-parekh force-pushed the modify-weather-sp-for-copy-files branch from 804824f to f8e8273 Compare July 20, 2026 12:43
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