Skip to content
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.

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

feat: populate .repo-metadata.json from highest version #2890

Merged
merged 6 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion library_generation/generate_composed_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def generate_composed_library(
base_arguments = __construct_tooling_arg(config=config)
owlbot_cli_source_folder = util.sh_util("mktemp -d")
os.makedirs(f"{library_path}", exist_ok=True)
for gapic in library.gapic_configs:
for gapic in library.get_sorted_gapic_configs():
build_file_folder = Path(f"{output_folder}/{gapic.proto_path}").resolve()
print(f"build_file_folder: {build_file_folder}")
gapic_inputs = parse_build_file(build_file_folder, gapic.proto_path)
Expand Down
75 changes: 75 additions & 0 deletions library_generation/model/gapic_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import re
from typing import Optional

ALPHA_VERSION = "alpha"
BETA_VERSION = "beta"


class GapicConfig:
Expand All @@ -22,3 +27,73 @@ class GapicConfig:

def __init__(self, proto_path: str):
self.proto_path = proto_path
self.version = self.__parse_version()

def is_stable(self):
return (
self.version
and (ALPHA_VERSION not in self.version)
and (BETA_VERSION not in self.version)
)

def get_version(self):
return self.version

def __parse_version(self) -> Optional[str]:
version_regex = re.compile(r"^v[1-9]+(p[1-9]+)*(alpha|beta)?.*")
for directory in self.proto_path.split("/"):
if version_regex.search(directory):
return directory
return None

def __eq__(self, other) -> bool:
if not isinstance(other, GapicConfig):
return False
return self.proto_path == other.proto_path

def __lt__(self, other) -> bool:
if not isinstance(other, GapicConfig):
raise ValueError(
f"Type {type(other)} can't be comparable " f"with GapicConfig."
)

self_version = self.get_version()
other_version = other.get_version()
self_dirs = self.proto_path.split("/")
other_dirs = other.proto_path.split("/")
# Case 1: if both of the configs don't have a version in proto_path,
# the one with lower depth is smaller.
if (not self_version) and (not other_version):
return len(self_dirs) < len(other_dirs)
# Case 2: if only one config has a version in proto_path, it is smaller
# than the other one.
if self_version and (not other_version):
return True
if (not self_version) and other_version:
return False
# Two configs both have a version in proto_path.
self_stable = self.is_stable()
other_stable = other.is_stable()
# Case 3, if only config has a stable version in proto_path, it is
# smaller than the other one.
if self_stable and (not other_stable):
return True
if (not self_stable) and other_stable:
return False
# Case 4, if two configs have a non-stable version in proto_path,
# the one with higher version is smaller.
# Note that using string comparison may lead unexpected result,
# e.g., v1beta10 is smaller than v1beta2.
# In reality, however, there's unlikely that a library has >10 beta
# versions.
if (not self_stable) and (not other_stable):
return self_version > other_version
# Two configs both have a stable version in proto_path.
# Case 5, if two configs have different depth in proto_path, the one
# with lower depth is smaller.
if len(self_dirs) != len(other_dirs):
return len(self_dirs) < len(other_dirs)
# Case 6, the config with higher stable version is smaller.
self_num = int(self_version[1:])
other_num = int(other_version[1:])
return self_num > other_num
7 changes: 5 additions & 2 deletions library_generation/model/library_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
from hashlib import sha1

from typing import List, Optional
from typing import Optional
from library_generation.model.gapic_config import GapicConfig


Expand All @@ -29,7 +29,7 @@ def __init__(
api_description: str,
name_pretty: str,
product_documentation: str,
gapic_configs: List[GapicConfig],
gapic_configs: list[GapicConfig],
library_type: Optional[str] = None,
release_level: Optional[str] = None,
api_id: Optional[str] = None,
Expand Down Expand Up @@ -80,6 +80,9 @@ def get_library_name(self) -> str:
"""
return self.library_name if self.library_name else self.api_shortname

def get_sorted_gapic_configs(self) -> list[GapicConfig]:
return sorted(self.gapic_configs)

def __eq__(self, other):
return (
self.api_shortname == other.api_shortname
Expand Down
122 changes: 122 additions & 0 deletions library_generation/test/model/gapic_config_unit_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import unittest

from library_generation.model.gapic_config import GapicConfig


class GapicConfigTest(unittest.TestCase):
def test_get_version_returns_none(self):
self.assertIsNone(GapicConfig(proto_path="example/dir1/dir2").get_version())

def test_get_version_returns_non_stable_version(self):
self.assertEqual(
"v2p2beta1",
GapicConfig(proto_path="example/dir1/dir2/v2p2beta1").get_version(),
)
self.assertEqual(
"v2beta1",
GapicConfig(proto_path="example/dir1/dir2/v2beta1").get_version(),
)

def test_get_version_returns_stable_version(self):
self.assertEqual(
"v20",
GapicConfig(proto_path="example/dir1/dir2/v20").get_version(),
)

def test_is_stable_with_no_version_returns_false(self):
self.assertFalse(
GapicConfig(proto_path="example/dir1/dir2/non_version").is_stable(),
)

def test_is_stable_with_non_stable_version_returns_false(self):
self.assertFalse(
GapicConfig(proto_path="example/dir1/dir2/v20alpha").is_stable(),
)
self.assertFalse(
GapicConfig(proto_path="example/dir1/dir2/v20beta2").is_stable(),
)

def test_is_stable_with_stable_version_returns_true(self):
self.assertTrue(
GapicConfig(proto_path="example/dir1/dir2/v30").is_stable(),
)

def test_compare_configs_without_a_version(self):
config_len_3 = GapicConfig(proto_path="example/dir1/dir2")
config_len_4 = GapicConfig(proto_path="example/dir1/dir2/dir3")
self.assertLess(
config_len_3,
config_len_4,
"config_len_3 should be smaller since it has a lower depth.",
)

def test_compare_configs_only_one_has_a_stable_version(self):
versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v1")
non_versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3")
self.assertLess(
versioned_config,
non_versioned_config,
"versioned_config should be smaller since it has a version.",
)

def test_compare_configs_only_one_has_a_non_stable_version(self):
non_stable_versioned_config = GapicConfig(
proto_path="example/dir1/dir2/dir3/dir4/v1beta"
)
non_versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3")
self.assertLess(
non_stable_versioned_config,
non_versioned_config,
"non_stable_versioned_config should be smaller since it has a version.",
)

def test_compare_configs_one_has_non_stable_and_one_has_stable_version(self):
stable_versioned_config = GapicConfig(
proto_path="example/dir1/dir2/dir3/dir4/v1"
)
non_stable_versioned_config = GapicConfig(proto_path="example/dir1/dir2/v2beta")
self.assertLess(
stable_versioned_config,
non_stable_versioned_config,
"stable_versioned_config should be smaller since it has a stable version.",
)

def test_compare_configs_two_have_non_stable_version(self):
v3p2beta = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v3p2beta")
v2p4beta = GapicConfig(proto_path="example/dir1/dir2/v2p4beta")
self.assertLess(
v3p2beta,
v2p4beta,
"v3p2beta should be smaller since it has a higher version.",
)

def test_compare_configs_two_have_stable_version_different_depth(self):
v3 = GapicConfig(proto_path="example/dir1/dir2/v3")
v4 = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v4")
self.assertLess(
v3,
v4,
"v3 should be smaller since it has lower depth",
)

def test_compare_configs_two_have_stable_version_same_depth(self):
v4 = GapicConfig(proto_path="example/dir1/dir2/v4")
v3 = GapicConfig(proto_path="example/dir1/dir2/v3")
self.assertLess(
v4,
v3,
"v4 should be smaller since it has a higher version",
)
27 changes: 27 additions & 0 deletions library_generation/test/model/library_config_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.
import unittest

from library_generation.model.gapic_config import GapicConfig
from library_generation.model.library_config import LibraryConfig


Expand All @@ -37,3 +38,29 @@ def test_get_library_returns_api_shortname(self):
gapic_configs=list(),
)
self.assertEqual("secret", library.get_library_name())

def test_get_sorted_gapic_configs_returns_correct_order(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we create a test for each of the scenario? [v1alpha1, v1], [v1, v2] etc. Because

  1. It would be much more readable. For example, just by reading the tests, I don't know why v1alpha1, v1, v2, admin_v2, non_versioned, v1beta1 is sorted to v2, v1, admin_v2, v1beta1, v1alpha1, non_versioned.
  2. We can also easily test more scenarios. e.g. scenarios of two non versioned libraries.

Copy link
Collaborator Author

@JoeWang1127 JoeWang1127 Jun 18, 2024

Choose a reason for hiding this comment

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

I added unit tests to verify comparison result of two gapic_configs.

I kept this test case though.

v1beta1 = GapicConfig(proto_path="google/spanner/v1beta1")
v1 = GapicConfig(proto_path="google/spanner/v1")
v1alpha1 = GapicConfig(proto_path="google/spanner/v1alpha")
v2 = GapicConfig(proto_path="google/spanner/v2")
admin_v2 = GapicConfig(proto_path="google/spanner/admin/v2")
non_versioned = GapicConfig(proto_path="google/spanner/type")
library = LibraryConfig(
api_shortname="secret",
name_pretty="",
product_documentation="",
api_description="",
gapic_configs=[v1alpha1, v1, v2, admin_v2, non_versioned, v1beta1],
)
self.assertEqual(
[
v2,
v1,
admin_v2,
v1beta1,
v1alpha1,
non_versioned,
],
library.get_sorted_gapic_configs(),
)
Loading