-
Notifications
You must be signed in to change notification settings - Fork 20
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
Support GitHub Enterprise for Kiln Release Notes Cmd #505
Conversation
Some questions we had while implementing this:
Known Issue To fix this, we need to change how the GitHub client used for release note generation is configured. Instead of passing the github-token as a flag we need to properly parse release source client for each release. |
4a95629
to
40e8979
Compare
story: TPCF-26493 Co-authored-by: Joe Eltgroth <jeltgroth@vmware.com>
40e8979
to
173c1aa
Compare
Co-authored-by: Joe Eltgroth <jeltgroth@vmware.com> story: TPCF-26493 this is required for generating tile release notes
33c9c56
to
b2ce0ba
Compare
@crhntr I tested the changes locally and here's what I noticed
|
I had hoped to keep it backwards compatible. Unfortunately, now we need a second configuration value (the GitHub enterprise host). We could add another environment variable for that, but we already have it configured in the Kilnfile. Also with the introduction of a second GitHub host, some BOSH Releases could come from enterprise and others from open source, each host requires a different token and we use the Kilnfile.lock to resolve which credentials to use. For local development, tile authors should have "~/.kiln/credentials.yml" configured so only automation scripts should break. |
|
If the Kilnfile uses variables (artifactory, aws...), they all need to be provided before you can parse it. Before this change, it was providing only one GitHub token from the environment or a flag. We did not use the release_sources section in the Kilnfile at all. With regard to the second issue, I wanted to run kiln against TAS to make sure I didn't break anything. Unfortunately, I was not able to get it to run locally due to the requirement on TrainStat. I can try to dig into the BOSH Release details missing but would prefer to pair with someone on RelEng since there is now a (a strictly RelEng) TrainStat dependency in the release-notes code path. |
Meeting scheduled for 10/02 |
Co-Authored-By: Ajita Jain <jajita@vmware.com>
9a1d40e
to
2f28fba
Compare
Co-Authored-By: Joe Eltgroth <joe.eltgroth@broadcom.com> Co-Authored-By: Christopher Hunter <hunter.christopher@icloud.com>
2f28fba
to
852058e
Compare
I ran this same command without your PR changes and using the current kiln release and it works fine. But when I test with the changes in the PR, I am seeing this error.
|
I tested the changes, and here's what I found.
But after passing all the variables, it worked as expected. |
Co-authored-by: Nick Rohn <nrohn@pivotal.io>
use "github_host" env var to fetch release notes for bosh releases bump Co-authored-by: Nick Rohn <nrohn@vmware.com>
96e337d
to
4e3e430
Compare
…atch the release sources to the release by org
0ffbd3e
to
6ff0858
Compare
6ff0858
to
ceaa24f
Compare
ceaa24f
to
430694d
Compare
The scope of this change is limited to the release-notes cmd for now and can be extended to other commands in the future commits/PR. |
[Co-authored by: Dave Walter <dave.walter@broadcom.com>]
Co-authored by: <Dave Walter dave.walter@broadcom.com>
story: TPCF-26493
Assumption:
If there are multiple release sources with the same org but different github access tokens, we will iterate through every release source which has org matching the org in the github_repository link specified for the release in the Kilnfile.