search

deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

feat(source_version): add SOURCE_VERSION env var #418

Closed aboyett closed 8 years ago

aboyett commented 8 years ago

heroku exposes the git commit hash of the revision being built in the SOURCE_VERSION environment variable. this commit implements this in the deis builder for git pushes.

see https://devcenter.heroku.com/changelog-items/630 for more info on heroku's implementation

deis-admin commented 8 years ago

Thanks for the contribution! Please ensure your commits follow our style guide. This code will be tested once a Deis maintainer reviews it.

deis-bot commented 8 years ago

@jeroenvisser101, @arschles and @aledbf are potential reviewers of this pull request based on my analysis of git blame information. Thanks @aboyett!

codecov-io commented 8 years ago

Current coverage is 47.14% (diff: 50.00%)

Merging #418 into master will increase coverage by 0.10%

@@             master       #418   diff @@
==========================================
  Files            26         26          
  Lines          1101       1105     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            518        521     +3   
- Misses          552        553     +1   
  Partials         31         31

Powered by Codecov. Last update 9c54cdd...05522a0

jeroenvisser101 commented 8 years ago

@aboyett looks good, maybe add some unit-tests?

aboyett commented 8 years ago

@jeroenvisser101 I augmented the existing tests to handle this new argument. Do you have any suggestions on what other tests you'd like to see added as part of this PR?

jeroenvisser101 commented 8 years ago

@aboyett ah, right, I think it's fine indeed. In #412 I added a method, but you're reusing gitSha.short() which has tests already. I think it's fine as is.

Joshua-Anderson commented 8 years ago

Jenkins, add to whitelist.

Joshua-Anderson commented 8 years ago

@aboyett Some other changes have been made to this bit of code, do you mind rebasing?

aboyett commented 8 years ago

Rebase complete, waiting on the e2e test to finish

felixbuenemann commented 8 years ago

If I understand this PR correctly, the SOURCE_VERSION environment variable is currently only available during build. Would it be possible to pass it on to the apps environment as well?

It would be great to pass as additional context in exception monitoring. I'm currently passing the WORKFLOW_RELEASE environment var as context, but that requires another roundtrip to look up the commit sha from deis releases to be able to see which version of the code this was.

Btw. it seems that heroku uses the environment variable HEROKU_SLUG_COMMIT for this, so it might make sense to emulate that variable in the slugbuilder. It would allow existing tools like the sentry client to detect the release without changes.

bacongobbler commented 8 years ago

@felixbuenemann mind filing a new issue so we can track that effort? :)

aboyett commented 8 years ago

My pull request attempted to replicate the behavior of Heroku which only makes the SOURCE_VERSION variable available during the compile phase[1]. I am curious to hear more about this HEROKU_SLUG_COMMIT env var as it doesn't appear to exist in cedar-14 based python buildpack generated slugs. If you create a new ticket, please reference this ticket so it is easy to find :smile:

[1] https://devcenter.heroku.com/articles/buildpack-api#bin-compile-summary

felixbuenemann commented 8 years ago

@aboyett It must be enabled on heroku, see Heroku Labs: Dyno Metadata.