chore: allow --api-path as option to generate cmd and generate per api/majorversion #3712

zhumin8 · 2025-03-18T21:26:47Z

This change allow generator to generate for a specified api-path. When "--api-path" is not specified, fallback to "--library-names", and behavior for existing command should not change.

For now, api-path should only take in path to api/version, e.g. google/cloud/functions/v2

--
Manual tests

python library_generation/cli/entry_point.py generate --output=/path/to/empty/output --generation-input=/path/to/gen-input --api-root=/path/to/googleapis --api-path=google/cloud/alloydb/v1

generate output

├── google-cloud-alloydb
│   ├── pom.xml                                                                            
│   ├── src                                                                                
│   │   ├── main                                                                           
│   │   │   ├──......                                                
│   │   │   │    └── v1  
├── google-cloud-alloydb-bom
├── grpc-google-cloud-alloydb-v1
├── owlbot.py
├── pom.xml
├── proto-google-cloud-alloydb-v1
├── README.md
└── samples

…static method in GenerationConfig.

sonarqubecloud · 2025-03-24T21:23:28Z

Quality Gate passed for 'gapic-generator-java-root'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sonarqubecloud · 2025-03-24T21:26:52Z

Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

diegomarquezp

Thanks for this PR. It will make local development much easier (no need to modify the gen config yaml anymore).

I left a couple of nits and a minor question.

diegomarquezp · 2025-03-25T16:30:43Z

hermetic_build/common/model/generation_config.py



-def __required(config: dict, key: str, level: str = LIBRARY_LEVEL_PARAMETER):
+def _required(config: dict, key: str, level: str = LIBRARY_LEVEL_PARAMETER):


Thanks for fixing this. Looks like this is recommended by our style guide. Just curious on any other changes of this kind in this PR?

diegomarquezp · 2025-03-25T16:34:16Z

hermetic_build/library_generation/generate_repo.py

+) -> list[LibraryConfig]:
+    """
+    Retrieves a copy of the LibraryConfig objects that contain the specified
+    target API path, removed other proto_path versions from LibraryConfig if any.


nit:

Suggested change

target API path, removed other proto_path versions from LibraryConfig if any.

target API path, without other proto_path versions that were not specified.

diegomarquezp · 2025-03-25T16:36:40Z

hermetic_build/library_generation/generate_repo.py

+    """
+    Retrieves a copy of the LibraryConfig objects that contain the specified
+    target API path, removed other proto_path versions from LibraryConfig if any.
+    proto_path that is dependency, not another version is kept.


nit:

Suggested change

proto_path that is dependency, not another version is kept.

dependency proto_paths are kept.

Hope you also find this wording easier to read. I think it's clear in the previous line that other proto_path versions are not considered.

JoeWang1127 · 2025-03-31T14:48:32Z

hermetic_build/common/model/generation_config.py

@@ -105,77 +105,77 @@ def __validate(self) -> None:
                )
            seen_library_names[library_name] = library.name_pretty

+    @staticmethod
+    def from_yaml(path_to_yaml: str) -> "GenerationConfig":


Are there any logical change in this method?

JoeWang1127 · 2025-03-31T14:52:23Z

hermetic_build/library_generation/cli/entry_point.py

+    show_default=True,
+    help="""
+    Path within the API root (e.g. googleapis) to the API to 
+    generate/build/configure etc. This is expected to be a major-versioned 


What's the difference of major-versioned and versioned?

JoeWang1127 · 2025-03-31T14:53:12Z

hermetic_build/library_generation/cli/entry_point.py

+    API directory, e.g. google/cloud/functions/v2.
+
+    Takes precedence over --library-names when specidied. 
+    If neither this or --api-path specified, all libraries in the 


Suggested change

If neither this or --api-path specified, all libraries in the

If neither --library-names or --api-path specified, all libraries in the

JoeWang1127 · 2025-03-31T14:58:12Z

hermetic_build/library_generation/cli/entry_point.py

+    """,
+)
+@click.option(
+    "--api-path",


I think this parameter is overlap with library-names and it's fine-gained than it.

Do you plan to remove library-names later?

JoeWang1127 · 2025-03-31T14:58:34Z

hermetic_build/common/utils/proto_path_utils.py

@@ -31,3 +31,21 @@ def find_versioned_proto_path(proto_path: str) -> str:
            idx = proto_path.find(version)
            return proto_path[:idx] + version
    return proto_path
+
+
+def ends_with_version(proto_path: str) -> bool:


Why do we need a setter method?

zhumin8 added 4 commits March 18, 2025 15:52

add target_api_path to generate_from_yaml(). refactor from_yaml() to …

35967ef

…static method in GenerationConfig.

fix test. Add mock to make sure skip generation.

d1c6801

adding api-path as click option to generate.

39cd5ea

pass down api-path. minor fixes.

6858033

product-auto-label bot added the size: l Pull request size is large. label Mar 18, 2025

zhumin8 added 3 commits March 20, 2025 16:55

update to account for api with proto/type dependency.

bb6cae6

fix test case.

2053749

add error handling for invalid input

2911ffd

zhumin8 marked this pull request as ready for review March 24, 2025 21:22

zhumin8 requested review from JoeWang1127 and diegomarquezp March 24, 2025 21:22

diegomarquezp approved these changes Mar 25, 2025

View reviewed changes

JoeWang1127 reviewed Mar 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: allow --api-path as option to generate cmd and generate per api/majorversion #3712

chore: allow --api-path as option to generate cmd and generate per api/majorversion #3712

zhumin8 commented Mar 18, 2025 •

edited

Loading

sonarqubecloud bot commented Mar 24, 2025

sonarqubecloud bot commented Mar 24, 2025

diegomarquezp left a comment

diegomarquezp Mar 25, 2025

diegomarquezp Mar 25, 2025

diegomarquezp Mar 25, 2025

JoeWang1127 Mar 31, 2025

JoeWang1127 Mar 31, 2025

JoeWang1127 Mar 31, 2025

JoeWang1127 Mar 31, 2025

JoeWang1127 Mar 31, 2025



		def __required(config: dict, key: str, level: str = LIBRARY_LEVEL_PARAMETER):
		def _required(config: dict, key: str, level: str = LIBRARY_LEVEL_PARAMETER):

	target API path, removed other proto_path versions from LibraryConfig if any.
	target API path, without other proto_path versions that were not specified.

	proto_path that is dependency, not another version is kept.
	dependency proto_paths are kept.

	If neither this or --api-path specified, all libraries in the
	If neither --library-names or --api-path specified, all libraries in the

chore: allow --api-path as option to generate cmd and generate per api/majorversion #3712

Are you sure you want to change the base?

chore: allow --api-path as option to generate cmd and generate per api/majorversion #3712

Conversation

zhumin8 commented Mar 18, 2025 • edited Loading

sonarqubecloud bot commented Mar 24, 2025

Quality Gate passed for 'gapic-generator-java-root'

sonarqubecloud bot commented Mar 24, 2025

Quality Gate passed for 'java_showcase_integration_tests'

diegomarquezp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhumin8 commented Mar 18, 2025 •

edited

Loading