-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Use test endpoint for datalabeling integration tests. #1803
Conversation
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.
Since there is no ReadMe for this sample, do the docs mention the new environment variable "DATALABELING_ENDPOINT"?
...beling/beta/cloud-client/src/main/java/com/example/datalabeling/CreateAnnotationSpecSet.java
Outdated
Show resolved
Hide resolved
|
||
try (DataLabelingServiceClient dataLabelingServiceClient = |
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.
Please add comments to each file, that this is unnecessary for users to do or something to that effect.
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.
+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.
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.
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.
Tests are not passing now that the variable typo was fixed. |
|
||
try (DataLabelingServiceClient dataLabelingServiceClient = |
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.
+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.
c3bc592
to
681182f
Compare
681182f
to
5a3ce71
Compare
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. |
@dzlier-gcp Perhaps we should submit this now so that the current integration tests stop, and then move it in a different PR |
* Use test endpoint for datalabeling integration tests. * Fix formatting * Fix typo * Fix formatting * Add quotes to variable * Add execeptions to method signitures.
* Use test endpoint for datalabeling integration tests. * Fix formatting * Fix typo * Fix formatting * Add quotes to variable * Add execeptions to method signitures.
No description provided.