-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Task/add create fileset entry #1525
Task/add create fileset entry #1525
Conversation
Changed to WIP, I will split the create entry and create entry group in two files. |
The PR is ready now @fhinkel |
// Construct the EntryGroup request to be sent by the client. | ||
const entryGroupRequest = { | ||
parent: datacatalog.locationPath(projectId, location), | ||
entryGroupId: entryGroupId, |
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.
We can use shorthand notation here:
const entryGroupRequest = {
parent: datacatalog.locationPath(projectId, location),
entryGroupId,
entryGroup,
};
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.
sure, changed.
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.
LGTM. Thanks!
The iot/manager test failing seem to be related to this change: Method name changed, but test from sample code was not updated. |
Yes, I can't merge this without manually diasbling CI. Let's wait a bit for the iot tests to get fixed. Sorry for the delay! |
Yea it makes sense, I sent a PR with a proposed fix: #1528 |
Thanks, LGTM. Once internal CL 277959432 merges, we can try again to run Kokoro. |
@tswast I solved the lint issue, but we need to enable Data Catalog API in the test environment: Can you help with that? |
I just enabled the API. @mesmacosta do you have permission to add the |
@fhinkel I don't |
Just gave you access. If you add the |
@fhinkel can I go ahead and merge? |
I would like to add samples for Data Catalog create_entry method, released on 1.3.0 @google-cloud/datacatalog version.