Skip to content

fix(commands): Add missing env vars for release name detection#2051

Merged
elramen merged 1 commit intomasterfrom
detect-release
May 22, 2024
Merged

fix(commands): Add missing env vars for release name detection#2051
elramen merged 1 commit intomasterfrom
detect-release

Conversation

@elramen
Copy link
Copy Markdown
Member

@elramen elramen commented Apr 30, 2024

Check env vars SENTRY_RELEASE and GAE_DEPLOYMENT_ID when detecting release name, as done by SDKs.

Fixes GH-2050

@elramen elramen requested a review from szokeasaurusrex April 30, 2024 15:34
@szokeasaurusrex
Copy link
Copy Markdown
Member

I am not sure whether this is very useful from the CLI context, especially because we are only using the detect_release function from some less-commonly used commands in the CLI:

  • releases propose-version: Kinda strange to return a user-configured environment variable as our suggestion here; user could just provide the releases new command (which does not read detect_release)
  • bash-hook: Used to monitor bash scripts for errors; I believe this command is "soft-deprecated" since we hide it from the sentry-cli --help output
  • send-event: Probably would be most useful here, this command sends individual events to Sentry

I am more uncertain about whether checking GAE_DEPLOYMENT_ID is necessary; can you please explain when you would envision this being useful?

@elramen
Copy link
Copy Markdown
Member Author

elramen commented May 3, 2024

Send-metric is going to auto-detect the release if it's not specified by the user. This is part of the metrics specification, i.e. the release tag should be added by default. Looking at the env vars checked by the Python SDK, detect_release_name() in sentry-cli checks all of them except the two added in this PR.

Especially SENTRY_RELEASE seems to be a common way of setting the release across different platforms/environments.

@elramen elramen self-assigned this May 14, 2024
Check env vars SENTRY_RELEASE and GAE_DEPLOYMENT_ID when detecting release name, as this is done by SDKs
/// Detects the release name for the current working directory.
pub fn detect_release_name() -> Result<String> {
// cordova release detection first.
// try SENTRY_RELEASE environment variable
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if the comment is almost the same as the code below it, just leave out the comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add missing env vars for release name detection

3 participants