Skip to content
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

Use getProviderUrl() instead of getHostName() when generating gitlab … #737

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Nov 11, 2024

What does this PR do?

Use getProviderUrl() instead of getHostName() when generating gitlab url parameter

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://issues.redhat.com/browse/CRW-7264

How to test this PR?

  1. Prepare a gitlab repository with .devfile.yaml file.
  2. Start a workspace from the gitlab repository url.

See: workspace starts successfully.

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@@ -29,7 +29,7 @@ class GitlabAuthorizingFileContentProvider extends AuthorizingFileContentProvide
@Override
protected boolean isPublicRepository(GitlabUrl remoteFactoryUrl) {
try {
urlFetcher.fetch(remoteFactoryUrl.getHostName() + '/' + remoteFactoryUrl.getSubGroups());
urlFetcher.fetch(remoteFactoryUrl.getProviderUrl() + '/' + remoteFactoryUrl.getSubGroups());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vinokurig so this method never actually worked since HostName is not URL, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is a leftover from a refactoring.

@ibuziuk ibuziuk requested a review from artaleks9 November 12, 2024 09:39
@ibuziuk
Copy link
Member

ibuziuk commented Nov 12, 2024

@artaleks9 wondering if we can add PR check for that ?

@artaleks9
Copy link
Contributor

artaleks9 commented Nov 12, 2024

| @artaleks9 wondering if we can add PR check for that ?

You mean a repository gitlab with .devfile.yaml ?
I think, yes

@ibuziuk
Copy link
Member

ibuziuk commented Nov 12, 2024

@dmytro-ndp @artaleks9 we should consider backporting to 7.92.x and 7.94.x

@artaleks9
Copy link
Contributor

Verified on Che with quay.io/eclipse/che-server:pr-737
on both gitlab server and GitLab.com provider.

The functionality works properly.

Copy link

openshift-ci bot commented Nov 12, 2024

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ibuziuk ibuziuk merged commit 1e31219 into main Nov 12, 2024
28 checks passed
@ibuziuk ibuziuk deleted the CRW-7264 branch November 12, 2024 18:25
@devstudio-release
Copy link

Build 3.18 :: server_3.x/371: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.18 :: server_3.x/371: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/8058 triggered

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.18 :: get-sources-rhpkg-container-build_3.x/8137: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 65882893 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants