Allow using factories with gitub repositories with the default branch that is not equal to "master" #18066

skabashnyuk · 2020-10-07T12:43:01Z

What does this PR do?

Allow using factories with gitub repositories with the default branch that is not equal to "master"
To get a devfile content used @benoitf 's idea #18066 (comment)

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Fixes #18044

How to test this PR?

Repo with default branch `main`

setup che with quay.io/skabashn/che-server:che18044 che server image
click on the link https://che-che.apps.cluster-17ef.17ef.example.opentlc.com/f?url=https://github.com/skabashnyuk/TestMainBranch.git
Expect result : Theia is able to clone https://github.com/dmytro-ndp/TestMainBranch.git with default branch main

Private or not existed GitHub repo

setup che with quay.io/skabashn/che-server:che18044 che server image
click on the link https://che-che.apps.cluster-17ef.17ef.example.opentlc.com//f?url=https://github.com/some/private.git
Expect result : Theia is trying to clone Github repo https://github.com/some/private.git

PR Checklist

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

Reviewers

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

che-bot · 2020-10-07T13:02:06Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-10-07T13:22:01Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

sleshchenko

Do we really need to evaluate the default branch here? Can't we just omit it?

che-bot · 2020-10-07T20:46:50Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

ℹ️ Use comment "[crw-ci-test]" to rerun happy path E2E test.

skabashnyuk · 2020-10-08T06:41:39Z

Do we really need to evaluate the default branch here? Can't we just omit it?

I'm afraid that is unlikely possible. we always (if the branch is not set) looking for a devfile in the root of the Github repository.

SkorikSergey · 2020-10-08T07:23:15Z

[ci-test]

che-bot · 2020-10-08T07:24:06Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Use command [ci-test] to rerun the test.

che-bot · 2020-10-08T07:30:13Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

ℹ️ Use comment "[crw-ci-test]" to rerun happy path E2E test.

sleshchenko · 2020-10-08T07:32:09Z

I did not take a look the code in details, I just saw a reported bug where there is no issue with devfile resolving but the resolved factory has initialized the main branch, which I assume we can omit and then theia just will do git clone and default branch will be used

che-bot · 2020-10-08T11:17:34Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

ℹ️ Use comment "[crw-ci-test]" to rerun happy path E2E test.

sparkoo

In general LGTM. Just some ideas to improvements and few questions.

...-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubURLParser.java

...re-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubUrl.java

benoitf · 2020-10-08T20:43:31Z

Can't we simply use HEAD instead of master in github url ?

Like https://raw.githubusercontent.com/eclipse/che/HEAD/devfile.yaml

Or with a repo with main branch:
https://raw.githubusercontent.com/spring-projects/spring-petclinic/HEAD/pom.xml

And omit branch in devfile if not specified ( so git clone use default as well)

sparkoo · 2020-10-09T06:35:08Z

Can't we simply use HEAD instead of master in github url ?

Nice, that would be much simpler. But don't we need the exact branch name later anyway?

che-bot · 2020-10-09T06:37:31Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

benoitf · 2020-10-09T06:38:49Z

@sparkoo : if it's not specified by the user, no as git clone operation will use what is configured by default
I think branch is only useful to retrieve the devfile with a raw URL

Using HEAD avoid the use of the API and its rate limit.

skabashnyuk · 2020-10-09T06:42:44Z

Can't we simply use HEAD instead of master in github url ?

will take a look.

… that is not equal to master Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>

skabashnyuk · 2020-10-09T14:22:53Z

[crw-ci-test]

che-bot · 2020-10-09T14:40:47Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:18066

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

benoitf · 2020-10-12T06:52:42Z

I think this fix should be back-ported to 7.20.x as more and more ppl may face this issue in the near future
WDYT @mkuznyetsov @skabashnyuk ?

che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Oct 7, 2020

sleshchenko reviewed Oct 7, 2020

View reviewed changes

skabashnyuk marked this pull request as ready for review October 8, 2020 11:07

skabashnyuk requested a review from nickboldt as a code owner October 8, 2020 11:07

skabashnyuk requested review from mshaposhnik, sparkoo and metlos and removed request for nickboldt October 8, 2020 11:07

sparkoo reviewed Oct 8, 2020

View reviewed changes

skabashnyuk force-pushed the che18044 branch from 94e0187 to 45c7d18 Compare October 9, 2020 07:50

Allow using factories with gitub repositories with the default branch…

fbd4a07

… that is not equal to master Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>

skabashnyuk force-pushed the che18044 branch from 45c7d18 to fbd4a07 Compare October 9, 2020 08:01

eclipse-che deleted a comment from che-bot Oct 9, 2020

sparkoo approved these changes Oct 9, 2020

View reviewed changes

benoitf approved these changes Oct 9, 2020

View reviewed changes

eclipse-che deleted a comment from che-bot Oct 9, 2020

skabashnyuk merged commit 25ddfb2 into eclipse-che:master Oct 12, 2020

skabashnyuk deleted the che18044 branch October 12, 2020 06:19

che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 12, 2020

azatsarynnyy mentioned this pull request Oct 15, 2020

Factory URL: devfile.yaml is not detected if the default branch is not master #18116

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow using factories with gitub repositories with the default branch that is not equal to "master" #18066

Allow using factories with gitub repositories with the default branch that is not equal to "master" #18066

skabashnyuk commented Oct 7, 2020 •

edited

Loading

che-bot commented Oct 7, 2020

che-bot commented Oct 7, 2020

sleshchenko left a comment

che-bot commented Oct 7, 2020

skabashnyuk commented Oct 8, 2020

SkorikSergey commented Oct 8, 2020

che-bot commented Oct 8, 2020

che-bot commented Oct 8, 2020

sleshchenko commented Oct 8, 2020

che-bot commented Oct 8, 2020

sparkoo left a comment •

edited

Loading

benoitf commented Oct 8, 2020 •

edited

Loading

sparkoo commented Oct 9, 2020

che-bot commented Oct 9, 2020

benoitf commented Oct 9, 2020

skabashnyuk commented Oct 9, 2020

skabashnyuk commented Oct 9, 2020

che-bot commented Oct 9, 2020

benoitf commented Oct 12, 2020

Allow using factories with gitub repositories with the default branch that is not equal to "master" #18066

Allow using factories with gitub repositories with the default branch that is not equal to "master" #18066

Conversation

skabashnyuk commented Oct 7, 2020 • edited Loading

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

Repo with default branch main

Private or not existed GitHub repo

PR Checklist

Reviewers

che-bot commented Oct 7, 2020

che-bot commented Oct 7, 2020

sleshchenko left a comment

Choose a reason for hiding this comment

che-bot commented Oct 7, 2020

skabashnyuk commented Oct 8, 2020

SkorikSergey commented Oct 8, 2020

che-bot commented Oct 8, 2020

che-bot commented Oct 8, 2020

sleshchenko commented Oct 8, 2020

che-bot commented Oct 8, 2020

sparkoo left a comment • edited Loading

Choose a reason for hiding this comment

benoitf commented Oct 8, 2020 • edited Loading

sparkoo commented Oct 9, 2020

che-bot commented Oct 9, 2020

benoitf commented Oct 9, 2020

skabashnyuk commented Oct 9, 2020

skabashnyuk commented Oct 9, 2020

che-bot commented Oct 9, 2020

benoitf commented Oct 12, 2020

skabashnyuk commented Oct 7, 2020 •

edited

Loading

Repo with default branch `main`

sparkoo left a comment •

edited

Loading

benoitf commented Oct 8, 2020 •

edited

Loading