build: adapt samples build configurations for monorepo#13033
Open
build: adapt samples build configurations for monorepo#13033
Conversation
Automates the adaptation of `samples_build.yaml` during library migration and fixes existing samples build files in the monorepo by injecting the mandatory `BUILD_SUBDIR` environment variable. - Creates the `update_samples_build.py` helper script to safely parse and inject `BUILD_SUBDIR` into `samples_build.yaml`. - Integrates the helper script into `migrate.sh` as part of the library migration flow. - Fixes `samples_build.yaml` files in already-migrated directories: `java-bigquery`, `java-bigquerystorage`, `java-datastore`, `java-logging`, and `java-storage`. Towards #12983
![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 introduces a new 'samples' job type to the Kokoro build script and updates several Cloud Build configurations to include a 'BUILD_SUBDIR' environment variable, supporting the monorepo migration. It also adds a Python script and updates the migration shell script to automate these YAML modifications. Feedback identifies critical issues with error propagation in the build script, where failures in module installation or Maven execution might be masked, and potential logic flaws in the Python script regarding the broad detection of existing variables and the placement of environment variables in multi-step Cloud Build configurations.
Comment on lines
+30
to
+46
| if 'BUILD_SUBDIR=' in content: | ||
| print(f"BUILD_SUBDIR already present in {file_path}. Skipping.") | ||
| sys.exit(0) | ||
|
|
||
| # Find the 'env:' line and insert 'BUILD_SUBDIR' after it | ||
| lines = content.splitlines() | ||
| new_lines = [] | ||
| modified = False | ||
|
|
||
| for line in lines: | ||
| new_lines.append(line) | ||
| match = re.match(r'^(\s*)env:\s*$', line) | ||
| if match and not modified: | ||
| indent = match.group(1) | ||
| # Insert BUILD_SUBDIR under env: | ||
| new_lines.append(f"{indent}- 'BUILD_SUBDIR={library_name}'") | ||
| modified = True |
Contributor
There was a problem hiding this comment.
The script has two potential issues with its logic:
- The check
if 'BUILD_SUBDIR=' in content:is very broad and might trigger a false positive if the string appears in a comment or a different context, causing the script to skip necessary updates. - It injects
BUILD_SUBDIRinto the firstenv:block it finds. In multi-step Cloud Build configs, the first step might be a setup task (e.g., cloning or decryption), while the actual build step (which needs the variable) is further down. Sinceenvvariables are step-scoped in Cloud Build, the variable must be added to the specific step that executes the build script.
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
1 participant
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.
Automates the adaptation of
samples_build.yamlduring library migration and fixes existing samples build files in the monorepo by injecting the mandatoryBUILD_SUBDIRenvironment variable.update_samples_build.pyhelper script to safely parse and injectBUILD_SUBDIRintosamples_build.yaml.migrate.shas part of the library migration flow.samples_build.yamlfiles in already-migrated directories:java-bigquery,java-bigquerystorage,java-datastore,java-logging, andjava-storage.This PR also deletes the unused Samples Kokoro job (cl/91227064).
Also see cl/911993739 for corresponding Cloud Build trigger updates.
Towards #12983