-
Notifications
You must be signed in to change notification settings - Fork 30
feat: detect google cloud run #161
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
base: main
Are you sure you want to change the base?
Conversation
src/providers.ts
Outdated
["CLOUDFLARE_PAGES", "CF_PAGES", { ci: true }], | ||
["CLOUDFLARE_WORKERS", "WORKERS_CI", { ci: true }], | ||
["CLOUDRUN", "K_SERVICE"], | ||
["CLOUDRUN_JOB", "CLOUD_RUN_JOB"], |
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.
For this i think we should add ci flag
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.
Also i guess we could have both as same name
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.
Another thing for clarity, maybe makes sense we call it GOOGLE_CLOUDRUN
?
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.
yeah, probably makes sense. as it is in AWS_AMPLIFY
or AZURE_...
["CIRRUS", "CIRRUS_CI"], | ||
["CLOUDFLARE_PAGES", "CF_PAGES", { ci: true }], | ||
["CLOUDFLARE_WORKERS", "WORKERS_CI", { ci: true }], | ||
["GOOGLE_CLOUDRUN", "K_SERVICE", { ci: true }], |
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.
Is K_SERVICE also for CI? (why we could need to make a distinction in this case between job and non job variant)
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.
I was very confused the by the ci: true
flag when editing the file. Neither Cloud Run or Cloud Run jobs are Continuous Integration tools. But from my understanding neither it is FIREBASE_APP_HOSTING
but it has the flag true
.
To the question, do we need to distinguish regular and jobs. There is a fine line there, they both run in Cloud Run. The purpose is different (one for requests, one for jobs). The environment variables are not exactly the same, neither the run time behaviour (Cloud Run Jobs runs "forever" the process, regular Cloud Run does not).
Do we want to expose it in this package, that's a good question. I didn't need it, and implemented it for "completion" but now I am not so sure. 🤔
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.
But from my understanding neither it is FIREBASE_APP_HOSTING but it has the flag true.
This variable is only set in their deployment CI/CD (not deployment runtime)
Add Cloud Run and Cloud Run jobs to the list of known providers. Using the env vars documented in:
https://cloud.google.com/run/docs/container-contract#jobs-env-vars
resolves #160