-
Notifications
You must be signed in to change notification settings - Fork 989
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
add github.url and github.api_url for ghes alpha #386
Conversation
// Temporary hack for GHES alpha | ||
var configurationStore = HostContext.GetService<IConfigurationStore>(); | ||
var runnerSettings = configurationStore.GetSettings(); | ||
if (!runnerSettings.IsHostedServer && !string.IsNullOrEmpty(runnerSettings.GitHubUrl)) |
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 think this needs to be string.IsNullOrEmpty(runnerSettings.GitHubUrl)
. This will be non-null if you're talking to hosted GitHub -- see where this is set.
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.
Or, is that supposed to be an ||
in the middle?
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.
good catch
@TingluoHuang when !runnerSettings.IsHostedServer
should we fallback to runnerSettings.ServerUrl
when GitHubUrl
is not defined?
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.
runnerSettings.ServerUrl is the pipelines url, pipelines.actions.githubusercontent.com/xxxxx
When GitHubUrl is not defined, it means we connected to Devfabric.
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.
ok i think the change here might be fine then, but additional change to figure out for registration against the appliance
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.
discussed offline w/ ting, leaving this way for now. When config against appliance works, will need an additional change to persist githuburl for that scenario.
For GHES alpha release, expose GHES server URL and API URL for actions (e.g. where checkout should clone from)