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

Resource patterns with wildcard '*' results in failing test for parse_path #701

Closed
busunkim96 opened this issue Nov 18, 2020 · 3 comments
Closed
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@busunkim96
Copy link
Contributor

Asset has a resource path annotated with pattern *

https://github.com/googleapis/googleapis/blob/f1e0ce4dbc5f59448f441093014d712af442984f/google/cloud/asset/v1/assets.proto#L96-L100

message Asset {
  option (google.api.resource) = {
    type: "cloudasset.googleapis.com/Asset"
    pattern: "*"
  };

This results in a unhelpful set of resource name helpers:

    @staticmethod
    def asset_path() -> str:
        """Return a fully-qualified asset string."""
        return "*".format()

    @staticmethod
    def parse_asset_path(path: str) -> Dict[str, str]:
        """Parse a asset path into its component segments."""
        m = re.match(r"^*$", path)
        return m.groupdict() if m else {}

The generated tests break because of the *.

def test_asset_path():
    expected = "*".format()
    actual = AssetServiceClient.asset_path()
    assert expected == actual


def test_parse_asset_path():
    expected = {}
    path = AssetServiceClient.asset_path(**expected)

    # Check that the path construction is reversible.
    actual = AssetServiceClient.parse_asset_path(path)
    assert expected == actual
                if not item or item[0][0] is AT:
>                   raise source.error("nothing to repeat",
                                       source.tell() - here + len(this))
E                   re.error: nothing to repeat at position 1

../../.pyenv/versions/3.8.1/lib/python3.8/sre_parse.py:668: error

The * pattern is uncommon and this method isn't actually helpful for parsing paths. I'm planning on deleting it from the generated output for now.

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Nov 19, 2020
@vam-google vam-google added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels Dec 1, 2020
@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Dec 1, 2020
@busunkim96 busunkim96 changed the title Resource pattern '*' results in failing test for parse_path Resource patterns with wildcard '*' results in failing test for parse_path Dec 29, 2020
@busunkim96
Copy link
Contributor Author

Similar issue with monitoring which has a path with **.

https://github.com/googleapis/googleapis/blob/69697504d9eba1d064820c3085b4750767be6d08/google/monitoring/v3/metric_service.proto#L39-L46

option (google.api.resource_definition) = {
  type: "monitoring.googleapis.com/MetricDescriptor"
  pattern: "projects/{project}/metricDescriptors/{metric_descriptor=**}"
  pattern: "organizations/{organization}/metricDescriptors/{metric_descriptor=**}"
  pattern: "folders/{folder}/metricDescriptors/{metric_descriptor=**}"
  pattern: "*"
  history: ORIGINALLY_SINGLE_PATTERN
};
    def metric_descriptor_path(project: str,) -> str:
        """Return a fully-qualified metric_descriptor string."""
        return "projects/{project}/metricDescriptors/{metric_descriptor=**}".format(
            project=project,
        )

    @staticmethod
    def parse_metric_descriptor_path(path: str) -> Dict[str, str]:
        """Parse a metric_descriptor path into its component segments."""
        m = re.match(
            r"^projects/(?P<project>.+?)/metricDescriptors/{metric_descriptor=**}$",
            path,
        )
        return m.groupdict() if m else {}
def test_metric_descriptor_path():
    project = "squid"

    expected = "projects/{project}/metricDescriptors/{metric_descriptor=**}".format(
        project=project,
    )
    actual = MetricServiceClient.metric_descriptor_path(project)
    assert expected == actual


def test_parse_metric_descriptor_path():
    expected = {
        "project": "clam",
    }
    path = MetricServiceClient.metric_descriptor_path(**expected)

    # Check that the path construction is reversible.
    actual = MetricServiceClient.parse_metric_descriptor_path(path)
    assert expected == actual

@vam-google
Copy link
Contributor

@busunkim96 Is this issue still relevant?

@busunkim96
Copy link
Contributor Author

This was fixed by #1089.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants