Apply GitHub server flag when resolving factory URL #701
Conversation
| @@ -78,6 +79,7 @@ public abstract class AbstractGithubURLParser { | |||
| this.apiClient = githubApiClient; | |||
| this.disableSubdomainIsolation = disableSubdomainIsolation; | |||
| this.providerName = providerName; | |||
| this.isGitHubServer = !isNullOrEmpty(oauthEndpoint); | |||
There was a problem hiding this comment.
why if there is no OAuth endpoint we treat is as GitHub server?
There was a problem hiding this comment.
If the oauth endpoint is not set, it means that the provider is GitHub SAAS by default. Otherwise we treat it as GitHub Server
There was a problem hiding this comment.
@vinokurig can we possibly extract this logic to the separate method and add javdoc clarity?
There was a problem hiding this comment.
Added some javadoc above the variable
| @@ -100,7 +102,7 @@ public boolean isValid(@NotNull String url) { | |||
| // Check if the given URL is a valid GitHub URL by reaching the endpoint of the GitHub | |||
| // server and analysing the response. This query basically only needs to be performed if the | |||
| // specified repository URL does not point to GitHub SaaS. | |||
| || (!GITHUB_SAAS_ENDPOINT.equals(endpoint) && isApiRequestRelevant(trimmedUrl)); | |||
| || (!isGitHubServer && isApiRequestRelevant(trimmedUrl)); | |||
There was a problem hiding this comment.
Unfortunately we can not test it because we use new GitHub client on each call
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
Verified on Eclipse Che with |
| @@ -78,6 +79,7 @@ public abstract class AbstractGithubURLParser { | |||
| this.apiClient = githubApiClient; | |||
| this.disableSubdomainIsolation = disableSubdomainIsolation; | |||
| this.providerName = providerName; | |||
| this.isGitHubServer = !isNullOrEmpty(oauthEndpoint); | |||
There was a problem hiding this comment.
| this.isGitHubServer = !isNullOrEmpty(oauthEndpoint); | |
| this.isGitHubServer = !isNullOrEmpty(oauthEndpoint) || GITHUB_SAAS_ENDPOINT.equals(oauthEndpoint); |
This way?
There was a problem hiding this comment.
Thank you for the suggestion, but I have commited: && !GITHUB_SAAS_ENDPOINT.equals(oauthEndpoint)
|
/retest |
| this.isGitHubServer = | ||
| !isNullOrEmpty(oauthEndpoint) && !GITHUB_SAAS_ENDPOINT.equals(oauthEndpoint); |
There was a problem hiding this comment.
I struggle to understand this line. Could it be extracted to a separate method with Javadoc?
There was a problem hiding this comment.
Added some javadoc above the variable
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: artaleks9, ibuziuk, tolusha, vinokurig The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
Build 3.16 :: server_3.x/350: Console, Changes, Git Data |
|
Build 3.16 :: sync-to-downstream_3.x/7330: Console, Changes, Git Data |
|
Build 3.16 :: push-latest-container-to-quay_3.x/4806: Console, Changes, Git Data |
|
Build 3.16 :: get-sources-rhpkg-container-build_3.x/7314: server : 3.x :: Build 62945718 : quay.io/devspaces/server-rhel8:3.16-12 |
|
Build 3.16 :: server_3.x/350: Upstream sync done; /DS_CI/sync-to-downstream_3.x/7330 triggered |
|
Build 3.16 :: update-digests_3.x/7174: Console, Changes, Git Data |
|
Build 3.16 :: update-digests_3.x/7174: No new images detected: nothing to do! |
What does this PR do?
Fix an issue when starting a workspace using a public GitHub Server repo fails.
Screenshot/screencast of this PR
What issues does this PR fix or reference?
https://issues.redhat.com/browse/CRW-6831
How to test this PR?
Start a workspace from a public GitHub Enterprise Server repository with a devfile.
See: workspace starts
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.