X Tutup
Skip to content

Add Java sample for creating custom entries in Data Catalog#2417

Closed
muhsinking wants to merge 1 commit intoGoogleCloudPlatform:masterfrom
muhsinking:master
Closed

Add Java sample for creating custom entries in Data Catalog#2417
muhsinking wants to merge 1 commit intoGoogleCloudPlatform:masterfrom
muhsinking:master

Conversation

@muhsinking
Copy link
Copy Markdown

@muhsinking muhsinking requested a review from a team March 16, 2020 17:37
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 16, 2020
Copy link
Copy Markdown
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

Tests are missing.

Copy link
Copy Markdown
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

PTAL

Comment on lines +58 to +59
// PermissionDeniedException or NotFoundException are thrown if
// Entry does not exist.
Copy link
Copy Markdown
Contributor

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?

Comment on lines +45 to +47
// 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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice

Comment on lines +92 to +94
} catch (Exception e) {
System.out.printf("\nCannot delete template: %s", tagTemplateName);
}
Copy link
Copy Markdown
Contributor

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.

Comment on lines +121 to +125
.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")
Copy link
Copy Markdown
Contributor

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.

Comment on lines +204 to +207
// 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());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really like this!

@muhsinking muhsinking closed this Mar 16, 2020
@muhsinking
Copy link
Copy Markdown
Author

Backing off on this change and waiting for the sample author to add tests. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

X Tutup