Skip to content

fix(hermetic-build): do not add release please comments on vertexai poms #12559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blakeli0 merged 1 commit into from
Apr 2, 2026
Merged

fix(hermetic-build): do not add release please comments on vertexai poms #12559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions sdk-platform-java/hermetic_build/library_generation/owlbot/src/fix_poms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ def update_parent_pom(filename: str, modules: List[module.Module]):
new_dependency.append(new_group)
new_dependency.append(new_artifact)
new_dependency.append(new_version)
new_dependency.append(comment)
if "vertexai" not in m.artifact_id:
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

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

medium

The logic to exclude 'vertexai' artifacts from receiving 'release please' comments is duplicated here and in update_bom_pom (line 314). Consider refactoring this into a shared constant or a helper function to improve maintainability and ensure consistency across different POM update functions.

new_dependency.append(comment)
new_dependency.tail = "\n "
dependencies.insert(1, new_dependency)

Expand Down Expand Up @@ -310,7 +311,8 @@ def update_bom_pom(filename: str, modules: List[module.Module]):
new_dependency.append(new_group)
new_dependency.append(new_artifact)
new_dependency.append(new_version)
new_dependency.append(comment)
if "vertexai" not in m.artifact_id:
new_dependency.append(comment)

if index == num_modules - 1:
new_dependency.tail = "\n "
Expand Down
47 changes: 47 additions & 0 deletions sdk-platform-java/hermetic_build/library_generation/tests/owlbot/fix_poms_unit_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,53 @@ def test_update_poms_group_id_does_not_start_with_google_correctly(self):
for sub_dir in sub_dirs:
self.__remove_file_in_subdir(ad_manager_resource, sub_dir)

def test_update_bom_pom_excludes_vertexai_comment(self):
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

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

medium

The new test case test_update_bom_pom_excludes_vertexai_comment only verifies the logic for update_bom_pom. Since the exclusion logic was also added to update_parent_pom, consider adding a corresponding test case to ensure that function also correctly handles 'vertexai' artifacts.

import tempfile
from library_generation.owlbot.src.poms.module import Module
from library_generation.owlbot.src.fix_poms import update_bom_pom
Comment on lines +39 to +41
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

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

medium

According to PEP 8, imports should be placed at the top of the file. Importing modules like tempfile, Module, and update_bom_pom inside a test method is generally discouraged as it can lead to redundant imports and reduced readability. Please move these to the top of the file.

References
  1. PEP 8: Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants. (link)


# Minimal XML structure
initial_xml = """<?xml version="1.0" encoding="utf-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0">
<dependencyManagement>
<dependencies>
</dependencies>
</dependencyManagement>
</project>"""

with tempfile.NamedTemporaryFile(mode="w+", delete=False) as tmp:
tmp.write(initial_xml)
tmp_path = tmp.name

try:
modules = [
Module(
group_id="com.google.cloud",
artifact_id="google-cloud-vertexai",
version="1.0.0",
release_version="1.0.0",
),
Module(
group_id="com.google.cloud",
artifact_id="google-cloud-datastore",
version="1.0.0",
release_version="1.0.0",
),
]

update_bom_pom(tmp_path, modules)

with open(tmp_path, "r") as f:
content = f.read()

self.assertNotIn("x-version-update:google-cloud-vertexai:current", content)
self.assertIn("x-version-update:google-cloud-datastore:current", content)

finally:
import os
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

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

medium

Similar to the imports at the beginning of the method, import os should be moved to the top of the file to adhere to PEP 8 standards.

References
  1. PEP 8: Imports are always put at the top of the file. (link)


os.unlink(tmp_path)

@classmethod
def __copy__golden(cls, base_dir: str, subdir: str):
golden = os.path.join(base_dir, subdir, "pom-golden.xml")
Expand Down
Loading