chore: always fully generate the repo if it is a non-monorepo or contains common protos #3162
Conversation
JoeWang1127
changed the title
JoeWang1127
changed the title
Can we leave this part to the nightly generation? |
When I changed the workflow, the workflow brings this change. It's not a manual commit. I think it's good because it verified the expected behavior. In addition, the nightly generation should only bring changes with respect to googleapis commit, not protoc change in Dockerfile. |
| # does special handling when a method is annotated with @main.command() | ||
| generate_impl( | ||
| baseline_generation_config_path=baseline_config_path, | ||
| current_generation_config_path=current_config_path, |
There was a problem hiding this comment.
Can we use the same config path? I think it should not matter if the two configs are the same?
There was a problem hiding this comment.
This test is verifying the list of changed library can be populated in a monorepo.
If we use two identical configs, the list should be empty, i.e., no changed library. I think this is fine as long as the list is not None.
I'll make the change.
| @@ -163,6 +163,15 @@ def __generate_repo_and_pr_description_impl( | |||
| ) | |||
|
|
|||
|
|
|||
| def __needs_full_repo_generation(config_change: ConfigChange) -> bool: | |||
There was a problem hiding this comment.
nit: private functions are recommended to have single underscores
There was a problem hiding this comment.
I changed the function name in this file.
We can gradually change functions in other files in future PRs.
There was a problem hiding this comment.
Thanks @diegomarquezp, TIL.
|
|
| @@ -66,13 +79,13 @@ def _run_command_and_get_sdout(self, command, **kwargs): | |||
| def test_get_grpc_version_with_no_env_var_fails(self): | |||
| # the absence of DOCKER_GRPC_VERSION will make this function to fail | |||
| result = self._run_command("get_grpc_version") | |||
| self.assertEquals(1, result.returncode) | |||
There was a problem hiding this comment.
assertEquals is deprecated so I changed it to assertEqual.
…ains common protos (#3162) In this PR: - Always perform a full generation in a non-monorepo or the repo contains common protos. - Secure generation workflow (use environment variable to avoid script injection), inspired by googleapis/java-bigtable#2317 This PR also brings changes in common protos and iam due to protoc updates (25.3 -> 25.4). --------- Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
In this PR:
This PR also brings changes in common protos and iam due to protoc updates (25.3 -> 25.4).