Skip to content

Use test endpoint for datalabeling integration tests. #1803

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

Merged
merged 7 commits into from
Dec 17, 2019

Conversation

dzlier-gcp
Copy link
Member

No description provided.

@dzlier-gcp dzlier-gcp requested a review from a team December 11, 2019 23:01
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 11, 2019
@dzlier-gcp dzlier-gcp requested a review from kurtisvg December 12, 2019 19:21
Copy link
Contributor

@averikitsch averikitsch left a comment

Choose a reason for hiding this comment

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

Since there is no ReadMe for this sample, do the docs mention the new environment variable "DATALABELING_ENDPOINT"?


try (DataLabelingServiceClient dataLabelingServiceClient =
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add comments to each file, that this is unnecessary for users to do or something to that effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. We should also add a comment why this is necessary for these snippets, and try to hide this portion from being staged in the documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It would be nice to still add a comment here (doesn't need to be in the staged version) to explain the extra code.

@averikitsch
Copy link
Contributor

Tests are not passing now that the variable typo was fixed.


try (DataLabelingServiceClient dataLabelingServiceClient =
Copy link
Contributor

Choose a reason for hiding this comment

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

+1. We should also add a comment why this is necessary for these snippets, and try to hide this portion from being staged in the documentation.

@dzlier-gcp
Copy link
Member Author

Justin suggested talking to Jeff Ching about moving this sample into the Datalabeling repo so that the tests are only triggered when an actual change to datalabeling goes through, instead of changing the endpoint we're targeting, so I'm going to check with him before submitting.

@kurtisvg
Copy link
Contributor

@dzlier-gcp Perhaps we should submit this now so that the current integration tests stop, and then move it in a different PR

@dzlier-gcp dzlier-gcp merged commit cb61467 into GoogleCloudPlatform:master Dec 17, 2019
ivanmkc pushed a commit that referenced this pull request Nov 3, 2022
* Use test endpoint for datalabeling integration tests.

* Fix formatting

* Fix typo

* Fix formatting

* Add quotes to variable

* Add execeptions to method signitures.
averikitsch pushed a commit that referenced this pull request Nov 9, 2022
* Use test endpoint for datalabeling integration tests.

* Fix formatting

* Fix typo

* Fix formatting

* Add quotes to variable

* Add execeptions to method signitures.
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.

4 participants