Skip to content
This repository was archived by the owner on Sep 5, 2023. It is now read-only.

fix: make datacatalog == datacatalog_v1#206

Merged
busunkim96 merged 6 commits intomasterfrom
fix-default-alias
Aug 13, 2021
Merged

fix: make datacatalog == datacatalog_v1#206
busunkim96 merged 6 commits intomasterfrom
fix-default-alias

Conversation

@busunkim96
Copy link
Copy Markdown
Contributor

Fixes #116

I have verified that v1beta1 -> v1 is additive, so this is not
a breaking change. See internal changelist 390485345 for the proto
level diff and successful run through the proto brekaing change detector

@busunkim96 busunkim96 requested a review from a team August 13, 2021 00:17
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 13, 2021
@product-auto-label product-auto-label Bot added the api: datacatalog Issues related to the googleapis/python-datacatalog API. label Aug 13, 2021
@busunkim96 busunkim96 requested a review from parthea August 13, 2021 00:18
@busunkim96
Copy link
Copy Markdown
Contributor Author

Hmm the failing samples seem to be v1beta1, which shouldn't be impacted by this change.

I see a mix of failures and passes on these tests on continuous builds on 8/12 and 8/13. Weirdly no periodic builds have failed.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Aug 13, 2021

@busunkim96 The periodic builds roll back to run against the last released version of the repo, and the samples were added in PR #78, which landed after the v3.4.0 release.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Aug 13, 2021

I just ran the v1beta1 samples locally, and see the same errors. Could the backend have added a new constraint? The Entry docs say:

Field Type Description
user_specified_system string This field indicates the entry's source system that Data Catalog does not integrate with. user_specified_system strings must begin with a letter or underscore and can only contain letters, numbers, and underscores; are case insensitive; must be at least 1 character and at most 64 characters long.

This patch gets the v1beta1 samples passing for me:

--- a/samples/v1beta1/conftest.py
+++ b/samples/v1beta1/conftest.py
@@ -86,9 +86,16 @@ def entry(client, entry_group_name):
         now.strftime("%Y%m%d%H%M%S"), uuid.uuid4().hex[:8]
     )
     entry = datacatalog_v1beta1.CreateEntryRequest
-    entry = client.create_entry(
-        request={"parent": entry_group_name, "entry_id": random_entry_id, "entry": {"type_": "DATA_STREAM", "name": "samples_test_entry"}}
-    )
+    request = {
+        "parent": entry_group_name,
+        "entry_id": random_entry_id,
+        "entry": {
+            "type_": "DATA_STREAM",
+            "user_specified_system": "sample_system",
+            "name": "samples_test_entry",
+        },
+    }
+    entry = client.create_entry(request=request)
     yield entry.name
     client.delete_entry(request={"name": entry.name})
 

@busunkim96
Copy link
Copy Markdown
Contributor Author

@tseaver Ah I forgot about the rolling back bit...

The failures for the various Python versions didn't happen at the same time strangely, the 3.7 continuous failed this morning, but 3.6 and 3.8 passed. 🤷‍♀️

@steffnay do you know of any recent changes to the Datacatalog v1beta1 surface?

@busunkim96
Copy link
Copy Markdown
Contributor Author

After searching around internally I found internal changelist 387097607, which supposed to fix bug 194281199.

Explanation provided is the bug:

Cannot set user-specified-system on update if it was not set at the moment of creation of a topic

If one tries to set a user-specified-system for the first time via updating an entry, an error is thrown:

  "error": {
    "code": 400,
    "message": "Changing managing system from MANUAL to ON_PREM is not allowed.",
    "status": "INVALID_ARGUMENT"
  }
}```

It sounds to me like allowing an entry of that type without user_specified_system was a bug, and would result in the backend failing on subsequent calls. I'll update the sample with that patch.

@busunkim96 busunkim96 requested review from a team and shollyman as code owners August 13, 2021 18:14
@busunkim96 busunkim96 merged commit aefe892 into master Aug 13, 2021
@busunkim96 busunkim96 deleted the fix-default-alias branch August 13, 2021 20:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: datacatalog Issues related to the googleapis/python-datacatalog API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1beta1 is being used as the default alias instead of v1

5 participants