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

fix: make datacatalog == datacatalog_v1 #206

Merged
merged 6 commits into from Aug 13, 2021
Merged

Conversation

busunkim96
Copy link
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 as a code owner 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
Copy link
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
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
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
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
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 shollyman and a team 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