-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add Java sample for creating custom entries in Data Catalog #2417
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
Conversation
lesv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are missing.
lesv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL
| // PermissionDeniedException or NotFoundException are thrown if | ||
| // Entry does not exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that this is here -- Care to explain why one or the other?
| // Initialize client that will be used to send requests. This client only needs to be created | ||
| // once, and can be reused for multiple requests. After completing all of your requests, call | ||
| // the "close" method on the client to safely clean up any remaining background resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
| } catch (Exception e) { | ||
| System.out.printf("\nCannot delete template: %s", tagTemplateName); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to be explicit.
| .setUserSpecifiedSystem("onprem_data_system") | ||
| .setUserSpecifiedType("onprem_data_asset") | ||
| .setDisplayName("My awesome data asset") | ||
| .setDescription("This data asset is managed by an external system.") | ||
| .setLinkedResource("//my-onprem-server.com/dataAssets/my-awesome-data-asset") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we have a preference for samples to use UUID's, possibly in conjunction w/ text. We often have multiple tests running at the same time.
| // AlreadyExistsException's are thrown if EntryGroup or Entry already exists. | ||
| // IOException's are thrown when unable to create the DataCatalogClient, | ||
| // for example an invalid Service Account path. | ||
| System.out.println("Error creating entry:\n" + e.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this!
|
Backing off on this change and waiting for the sample author to add tests. Thanks! |
This will replace the inline sample at: https://cloud.google.com/data-catalog/docs/how-to/custom-entries#data-catalog-custom-entry-java