Conversation
|
😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details. |
b9d0d06 to
57923f5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1031 +/- ##
==========================================
+ Coverage 81.39% 81.67% +0.27%
==========================================
Files 69 69
Lines 14547 14566 +19
==========================================
+ Hits 11841 11897 +56
+ Misses 2706 2669 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
57923f5 to
1d1d689
Compare
| (CIPlatform::GoogleCloudBuild, CIPlatform::JenkinsPipeline) | ||
| | (CIPlatform::JenkinsPipeline, CIPlatform::GoogleCloudBuild) => { | ||
| CIPlatform::GoogleCloudBuild | ||
| } |
There was a problem hiding this comment.
Because either Jekins or Google Cloud Build may provide BUILD_ID we need to prioritize Google Cloud Build if we detect it
There was a problem hiding this comment.
IIUC the CIPlatform will only be anything other than Unknown if the current env var we're interrogating / trying to prioritize off of is a ci_platform_env_key (based on the CIPlatform::from() here) - which only Jenkins's ci_platform_env_key is BUILD_ID. So perhaps this isn't explicitly needed? Or I'm misinterpreting
There was a problem hiding this comment.
The problem is that we don't guarantee the order of env vars we receive. So we could get BUILD_ID before TRIGGER_NAME. If we get both, then we need to decide which one to use: in the case of Jenkins and Google Cloud Build, Google Cloud Build can have BUILD_ID, but Jenkins will not have TRIGGER_NAME, so it's easy to prioritize Google Cloud Build
There was a problem hiding this comment.
Also, my test would fail without this 😝
| /// https://docs.cloud.google.com/build/docs/configuring-builds/substitute-variable-values | ||
| pub const GOOGLE_CLOUD_BUILD: &str = "TRIGGER_NAME"; |
There was a problem hiding this comment.
Google Cloud Build doesn't have any built-in, default provided env vars. This seemed like the most unique and one that is always available (since this is how you trigger a build)
| (CIPlatform::GoogleCloudBuild, CIPlatform::JenkinsPipeline) | ||
| | (CIPlatform::JenkinsPipeline, CIPlatform::GoogleCloudBuild) => { | ||
| CIPlatform::GoogleCloudBuild | ||
| } |
There was a problem hiding this comment.
IIUC the CIPlatform will only be anything other than Unknown if the current env var we're interrogating / trying to prioritize off of is a ci_platform_env_key (based on the CIPlatform::from() here) - which only Jenkins's ci_platform_env_key is BUILD_ID. So perhaps this isn't explicitly needed? Or I'm misinterpreting
Took some diffing to figure this one out
https://docs.cloud.google.com/build/docs/configuring-builds/substitute-variable-values
Also, these are not automatically provided. We'll have to guide users to passing the necessary env vars 😬
Google Cloud Build setup is very involved, so I haven't attempted at setting this up myself.