Handle URLs from GitHub Enterprise managed orgs #2098
Conversation
Fixed checks to include URLs with "ghe" not just github.com. Fixes URL parser to provide API URLs of the form assigned to GitHub Enterprise managed organisations.
| is_github_enterprise <- function(url) { | ||
| url <- get_baseurl(url) | ||
| url <- normalize_host(url) | ||
| grepl("ghe.com$", url) |
There was a problem hiding this comment.
Do you have any concrete info on how universally this applies to GHE? Like, is this scheme for the hostname forced on you or is it a default or recommended? I've long wanted to better account for GHE and dragged my feet due to my perception that this could be anything.
There was a problem hiding this comment.
For example, in #1897, the host is ghe-gsk-prod.metworx.com. So I am inclined to believe that full GHE support will need to make this configurable.
There was a problem hiding this comment.
I didn't stand-up the environment, so I can't be 100% certain, but my understanding is the following.
- Those using the Free, Pro and Team versions of GitHub will use "api.github.com" as normal.
- Those using GitHub Enterprise Cloud will use "api.SUBDOMAIN.ghe.com".
- Those using GitHub Enterprise Server will be self-hosting and so their API URL can be anything (as I assume is happening in the example of
github_remote_list()not picking up GHE host #1897 above).
To confirm item (2) above, you can have a look at this page in the GitHub API documentation which says the following in the first section:
If you access GitHub at GHE.com, replace api.github.com with your enterprise's dedicated subdomain at api.SUBDOMAIN.ghe.com.
If that's true, then you are correct in that the changes in this PR don't amount to full GHE support, because they don't account for self-hosted GitHub Enterprise server. And I agree that the only way to properly account for it would be to make it configurable. But it does feel to me like "ghe.com" is standard enough that it would be nice to account for it automatically without users having to set their own environment variable. Allowing a configurable override would of course be a sensible next step.
There was a problem hiding this comment.
Thanks for the extra info! So I gather that you have GitHub Enterprise Cloud? It does seems like accomodating that would be considerably easier than full configurability. I'm about to be travelling, but I'll come back and review this PR.
There was a problem hiding this comment.
So I gather that you have GitHub Enterprise Cloud?
That's correct yes.
I'm about to be travelling, but I'll come back and review this PR.
Safe travels! And thanks for the prompt response!
My organisation recently set itself up on a managed GitHub Enterprise organisation. Our new URL is of the form "https://my-cool-org.ghe.com". This PR makes the changes required for usethis to recognise this kind of URL as a GitHub one and parse it to find the appropriate API URL, which is of the form "https://api.my-cool-org.ghe.com".
My immediate use case was trying
usethis::use_pkgdown_github_pages()and I can confirm that this function now works with the proposed changes.