-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Update Cloud tasks samples #1211
Conversation
appengine-java8/tasks/README.md
Outdated
@@ -79,23 +73,19 @@ Create a task, targeted at the `/tasks/create` endpoint, with a payload specifie | |||
|
|||
``` | |||
mvn exec:java -Dexec.mainClass="com.example.task.CreateTask" \ | |||
-Dexec.args="--project-id $GOOGLE_CLOUD_PROJECT \ | |||
-Dexec.args="--project-id $PROJECT_ID \ |
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.
Why is this being changed? It's GOOGLE_CLOUD_PROJECT in all the samples I can think of, though I wouldn't be surprised if there were inconsistencies. Was there guidance on this that I missed?
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.
Trying to make it consistent between the samples; there are many inconsistencies between repos/samples. I don't think there is an official call.
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.
There are documentation pages that refer to GOOGLE_CLOUD_PROJECT env var though, including it being required for AEF, where I haven't seen any doc pages using just PROJECT_ID (aside from as a placeholder in CLI args or urls). That seems to only exist in samples.
You're right that the inconsistencies should be addressed though. If you're looking to remove one in favor of the other, though, this is the wrong direction - I found 39 examples of System.getenv("GOOGLE_CLOUD_PROJECT")
in the project, versus 15 examples of System.getenv("PROJECT_ID")
. Considering the first is referred to as an env var in our documentation, I'd prefer leaving this the way it was.
try (CloudTasksClient client = CloudTasksClient.create()) { | ||
|
||
// TODO(developer): Uncomment these lines and replace with your values. | ||
// String project = "my-project-id"; |
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.
Vars should be projectId
and queueName
to match below if this is kept as is. But uncommenting these lines would be a compile error because these fields are all declared up above. In addition, they are all pulled from CLI opts so the user is presumably already supplying them if they are running the CLI, unless this comment is more aimed at docs readers than people running the sample.
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.
Trying to comprise between the code snippet for the docs and running the sample with the CLI. Changing the comment to "Variables provided by the CLI."
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.
Last question, are pull queues not being supported anymore? I may have missed something about them being deleted.
Pull queues are removed from the beta launch. |
* update sample * remove pull queues * updates for beta * Change comment * revert env var name
* update sample * remove pull queues * updates for beta * Change comment * revert env var name
…1.0 (#1211) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:libraries-bom](https://cloud.google.com/java/docs/bom) ([source](https://togithub.com/googleapis/java-cloud-bom)) | `26.0.0` -> `26.1.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/compatibility-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/confidence-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-automl). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTguMCIsInVwZGF0ZWRJblZlciI6IjMyLjE1OC4wIn0=-->
…1.0 (#1211) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:libraries-bom](https://cloud.google.com/java/docs/bom) ([source](https://togithub.com/googleapis/java-cloud-bom)) | `26.0.0` -> `26.1.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/compatibility-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/confidence-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-automl). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTguMCIsInVwZGF0ZWRJblZlciI6IjMyLjE1OC4wIn0=-->
…1.0 (#1211) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:libraries-bom](https://cloud.google.com/java/docs/bom) ([source](https://togithub.com/googleapis/java-cloud-bom)) | `26.0.0` -> `26.1.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/compatibility-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/26.1.0/confidence-slim/26.0.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-automl). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTguMCIsInVwZGF0ZWRJblZlciI6IjMyLjE1OC4wIn0=-->
No description provided.