Repository navigation
feat(notebooks): add Notebooks API v2 Workbench instance samples and remove deprecated content - #14755
Open
iennae wants to merge 5 commits into
Open
feat(notebooks): add Notebooks API v2 Workbench instance samples and remove deprecated content#14755iennae wants to merge 5 commits into
iennae wants to merge 5 commits into
Conversation
…remove legacy v1/ML Engine content Problem: The notebooks/ directory contained an empty notebooks/samples/ skeleton referencing the deprecated Notebooks API v1 Apiary guide, a legacy tutorial targeting the decommissioned Cloud ML Engine API (ml.googleapis.com), and zero Python client library samples for managing Workbench instances with the Notebooks API v2 (google-cloud-notebooks). Solution: - Remove the deprecated notebooks/tutorials/cloud-ml-engine/ tutorial and empty notebooks/samples/ directory. - Update notebooks/README.md to document Workbench instances and link to notebooks/v2/. - Add 24 atomic Python client library samples under notebooks/v2/ using google.cloud.notebooks_v2 (google-cloud-notebooks==1.18.0) covering instance lifecycle, configuration updates, upgradability checks, rollbacks, diagnostics, GPU/custom container/CMEK/Shielded VM creation, Security Blueprint hardening (private VPC, single-user custom service account, disabled root/downloads/terminal), and IAM policy management. - Add unit tests in notebooks/v2/instances_test.py along with requirements.txt, requirements-test.txt, and noxfile_config.py.
product-auto-label
Bot
added
api: notebooks
|
Here is the summary of changes. You are about to add 24 region tags.
This comment is generated by snippet-bot.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes outdated AI Platform Notebooks samples and introduces a comprehensive set of atomic Python code samples and tests for the Notebooks API v2 (Workbench instances). The feedback highlights a critical issue in set_instance_iam_policy.py where setting an IAM policy directly can overwrite existing bindings, recommending a read-modify-write pattern instead. Additionally, it suggests safer region extraction from location_id in create_instance_cmek.py and create_instance_private_vpc.py to handle region-level inputs, and recommends making the --project-id argument optional in list_instances.py by inferring it from Application Default Credentials.
…er-sample tests, and use IAM read-modify-write Problem: 1. The initial Notebooks API v2 samples included inline argparse / __main__ blocks inside the sample files, placed region tags after import sys, hardcoded example-project in resource paths, and used a single combined instances_test.py file. 2. test_instance_iam_permissions.py started with 'test_', causing pytest to collect the sample function itself as a test during CI runs. 3. set_instance_iam_policy.py constructed a new Policy object with a single binding instead of reading and modifying the existing IAM policy. 4. notebooks/v2/noxfile_config.py did not align ignored_versions with the repository root noxfile_config.py. Solution: 1. Refactor all 24 samples in notebooks/v2/ to pure functions enclosed by region tags at the top of each file, consistently referring to 'Workbench instance' in comments/docstrings and interpolating project_id across all resource URIs. 2. Rename test_instance_iam_permissions.py to check_instance_iam_permissions.py (notebooks_v2_check_instance_iam_permissions) and split instances_test.py into 24 separate <sample_name>_test.py files (48 unit tests total). 3. Update set_instance_iam_policy.py to use the read-modify-write pattern (get_iam_policy -> policy.bindings.append -> set_iam_policy) and harden region_id extraction in create_instance_cmek.py, create_instance_private_vpc.py, and create_instance_hardened.py. 4. Update notebooks/README.md and notebooks/v2/README.md with enterprise security and administration positioning for Workbench instances, and align notebooks/v2/noxfile_config.py with the root configuration.
…tests/ Problem: 1. Placing all 24 per-sample unit test files directly in notebooks/v2/ cluttered the top-level sample directory. 2. When GitHub Actions lint-changed.sh runs flake8 from the repository root, top-level bare module imports in notebooks/v2/ were not recognized as local application imports by flake8-import-order. Solution: 1. Move all 24 <sample_name>_test.py files into notebooks/v2/tests/ and add package __init__.py files. 2. Use relative imports (from .. import <sample_name>) in notebooks/v2/tests/*_test.py so flake8-import-order and pytest succeed whether invoked from the repository root or from notebooks/v2/.
Problem: The v2 unit tests mostly asserted on stdout, which only echoed fields from the mock response that each test built itself. A sample could drop a security setting (public IP, root access, CMEK, instance owners), send a wrong update mask, hardcode a project or region in a resource path, or skip the IAM read-modify-write append, and its test would still pass. Solution: - Each success-path test now reads the request the sample passed to the mocked client (call_args.kwargs["request"]) and checks resource names, interpolated paths (KMS key, VPC network and subnet, snapshot, service account), security-relevant fields, and update mask paths. - Tests use PROJECT_ID=test-project and LOCATION_ID=us-west1-b instead of the sample defaults, so hardcoded projects or regions fail the test. - set_instance_iam_policy_test checks that the existing binding and etag from get_iam_policy are kept and the new binding is appended, and that set_iam_policy is never called when the read fails. Planted bugs to confirm the new assertions catch them: dropping notebook-disable-root, enabling public IP, skipping the IAM append, hardcoding the region, hardcoding the project in the KMS key and snapshot path, a wrong update mask, and dropping instance_owners. All 8 were caught.
- Create and start samples: note that the operation completes before the instance is ready, and point to get_instance to wait for ACTIVE. - update_instance_metadata: print the updated key, the image version, and the metadata key count instead of the full metadata map. - IAM policy samples: print the etag as base64 instead of raw bytes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modernizes the
notebooks/sample directory for Workbench instances (Notebooks API v2) and removes decommissioned legacy content:Remove Deprecated Legacy Content:
notebooks/tutorials/cloud-ml-engine/(legacy Cloud ML Engine tutorial using Python 2/3googleapiclient.discovery.build('ml', 'v1')andsklearn.externals.joblib).notebooks/samples/(legacy empty v1 placeholder directory withtensorflow==2.21.0).notebooks/rendered/,notebooks/tutorials/bigquery/, andnotebooks/tutorials/vision/.Add 24 Notebooks API v2 (
google.cloud.notebooks_v2) Samples innotebooks/v2/:list_instances.py(notebooks_v2_list_instances)get_instance.py(notebooks_v2_get_instance)create_instance.py(notebooks_v2_create_instance)stop_instance.py(notebooks_v2_stop_instance)start_instance.py(notebooks_v2_start_instance)reset_instance.py(notebooks_v2_reset_instance)delete_instance.py(notebooks_v2_delete_instance)update_instance_machine_type.py(notebooks_v2_update_instance_machine_type)update_instance_metadata.py(notebooks_v2_update_instance_metadata)update_instance_shielded_config.py(notebooks_v2_update_instance_shielded_config)check_instance_upgradability.py(notebooks_v2_check_instance_upgradability)upgrade_instance.py(notebooks_v2_upgrade_instance)rollback_instance.py(notebooks_v2_rollback_instance)diagnose_instance.py(notebooks_v2_diagnose_instance)create_instance_gpu.py(notebooks_v2_create_instance_gpu)create_instance_custom_container.py(notebooks_v2_create_instance_custom_container)create_instance_cmek.py(notebooks_v2_create_instance_cmek)create_instance_confidential.py(notebooks_v2_create_instance_confidential)create_instance_private_vpc.py(notebooks_v2_create_instance_private_vpc)create_instance_single_user.py(notebooks_v2_create_instance_single_user)create_instance_hardened.py(notebooks_v2_create_instance_hardened)get_instance_iam_policy.py(notebooks_v2_get_instance_iam_policy)set_instance_iam_policy.py(notebooks_v2_set_instance_iam_policy)check_instance_iam_permissions.py(notebooks_v2_check_instance_iam_permissions)Testing & Documentation:
<sample_name>_test.pyunit test files (48 tests total covering both success and error paths),requirements.txt,requirements-test.txt, andnoxfile_config.py.notebooks/README.mdandnotebooks/v2/README.mdwith enterprise security and administration positioning for Workbench instances.