Skip to content
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.

Sign up for GitHub

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

feat: fix task queue trigger for updating reports post release #801

Merged
merged 3 commits into from
Nov 11, 2024
Merged

feat: fix task queue trigger for updating reports post release #801

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4f7d29
fix: adding location
cka-y Nov 6, 2024
87c901d
fix: adding location
cka-y Nov 6, 2024
f1a8ac7
fix: adding location
cka-y Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/validator-update.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ jobs:
- name: Create task to run cloud function
run: |
gcloud tasks create-http-task \
--queue=update_validation_report_task_queue \
--queue=update-validation-report-task-queue \
Copy link
Contributor

@jcpitre jcpitre Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think having 2 ways to name the queue in main.tf (with underscore and dash) is just confusing. Why can't both be the same?

Or maybe the resource name should be: update_validation_report_task_queue_tf_name or something like this since I think it's only used within terraform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, all our resources are named using underscores, like name_of_resource, whereas individual elements are named with hyphens, such as name-of-element (this naming convention also applies to Python functions). However, when I try to name the queue as name_of_queue, GCloud throws an error.
The gcloud system requires queue IDs to contain only letters, numbers, or hyphens (-), and restricts the use of underscores.

Copy link
Contributor

@jcpitre jcpitre Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

FYI there's at least one case where the underscore is used in both  "names" (See there). But it's not a queue.

What about naming them differently? AFAIK the name on the resource line (update_validation_report_task_queue) is used only within terraform. So we could have some kind of suffix.

In any case I guess this might be out of scope for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I guess it would be valuable to go over the naming convention in terraform. Issue created: #802 -- feel free to add some details.

--url=https://${ENVIRONMENT}-${PROJECT_ID}.cloudfunctions.net/update-validation-report \
--schedule-time=$(date -u -d "+24 hours" +%Y-%m-%dT%H:%M:%SZ) \
--oidc-service-account-email=${DEPLOYER_SERVICE_ACCOUNT}
--oidc-service-account-email=${DEPLOYER_SERVICE_ACCOUNT} \
--location=${{ vars.MOBILITY_FEEDS_REGION }}
Loading