Skip to content

feat(linter): add automated migration for gsutil to gcloud storage#4503

Open
amiragamalyassin wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
amiragamalyassin:patch-1
Open

feat(linter): add automated migration for gsutil to gcloud storage#4503
amiragamalyassin wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
amiragamalyassin:patch-1

Conversation

@amiragamalyassin
Copy link
Copy Markdown

This PR updates the notebook_template_review.py script to support the ongoing migration from gsutil to gcloud storage within Vertex AI notebooks.

Changes:
Added ERROR_GSUTIL_DEPRECATED (103) to the ErrorCode enum.
Updated the substitutions dictionary in replace_cl to automatically convert deprecated storage commands to the modern gcloud storage CLI.
These updates ensure that all sample notebooks align with current Google Cloud performance standards and best practices.

@amiragamalyassin amiragamalyassin requested a review from a team as a code owner April 17, 2026 05:12
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 introduces a new error code for deprecated gsutil usage and adds substitutions to migrate gsutil commands to gcloud storage. However, several issues were identified: a syntax error was introduced in the substitutions dictionary due to a malformed entry and a duplicate key, the new error code is currently unused, and the gsutil substitutions in the replace_cl function do not yet affect the actual notebook content as intended.

Comment thread notebooks/notebook_template_review.py Outdated
Comment on lines 1305 to 1307
'{{vertex_nas_name}}',
'Neural Architectural Search': '{{vertex_nas_name_short}}',
'Vertex Workbench': '{{vertex_workbench_name}}',
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.

critical

This change introduces a SyntaxError because line 1305 is a single string literal instead of a key-value pair within the dictionary. Additionally, line 1306 is a duplicate of line 1304. These lines should be removed.

Suggested change
'{{vertex_nas_name}}',
'Neural Architectural Search': '{{vertex_nas_name_short}}',
'Vertex Workbench': '{{vertex_workbench_name}}',
'Vertex Workbench': '{{vertex_workbench_name}}',

Comment on lines +1321 to +1323
'gsutil ': 'gcloud storage ',
'!gsutil ': '!gcloud storage ',
'! gsutil ': '! gcloud storage ',
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

The replace_cl function is currently only used for generating the discoverability index (add_index) and does not affect the actual notebook content. If the goal is to automatically migrate gsutil commands in the notebooks, these substitutions need to be applied to the notebook cells during the validation/fix process. Also, consider if the trailing spaces in the keys (e.g., 'gsutil ') might miss cases where the command is followed by a newline or tab.

# git, colab and workbench link required
# links must be valid links
# Storage migration rules
ERROR_GSUTIL_DEPRECATED = 103, # Warning for deprecated gsutil usage
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.

medium

The error code ERROR_GSUTIL_DEPRECATED is defined but not used anywhere in the script. To properly implement this feature, you should add logic to one of the NotebookRule classes to detect gsutil usage and report this error.

Added StorageMigrationRule to validate and migrate deprecated gsutil commands to gcloud storage.
@amiragamalyassin
Copy link
Copy Markdown
Author

Thank you for the detailed feedback. I have pushed a follow-up commit that addresses all identified issues:
Fixed Syntax Errors: Cleaned up the substitutions dictionary by removing the malformed string literal and duplicate keys.
Active Implementation: I've implemented a new StorageMigrationRule class that scans notebook code cells for gsutil usage.
Usage of Error Code: The ERROR_GSUTIL_DEPRECATED (103) is now actively reported by the validator.
Automated Content Fix: The migration to gcloud storage now directly updates the notebook's cell content when the --fix flag is provided, ensuring compliance across all samples.
I've verified the logic, and it's now ready for another review. Thanks!

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