Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Update to upstream 7.4.0 #1701

Closed
wants to merge 8 commits into from
Closed

Update to upstream 7.4.0 #1701

wants to merge 8 commits into from

Conversation

amisevsk
Copy link
Collaborator

What does this PR do?

Update to upstream 7.4.0

  • Port changes in 7.4.0
  • Update dashboard (disable devfile editor autosave)

@centos-ci
Copy link
Collaborator

@centos-ci
Copy link
Collaborator

@amisevsk RH-Che [build 1974] has been successfully deployed and tested.

deployment: http://rhche-prcheck-1701.devtools-dev.ext.devshift.net
console: https://ci.centos.org/view/Devtools/job/devtools-rh-che-rh-che-prcheck-dev.rdu2c.fabric8.io/1974/console

Copy link
Member

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

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

Just let's merge it after 7.3.2 will be on produciton

@ibuziuk ibuziuk mentioned this pull request Nov 18, 2019
@amisevsk amisevsk changed the title [WIP] Update to upstream 7.4.0 Update to upstream 7.4.0 Nov 26, 2019
@amisevsk
Copy link
Collaborator Author

Removed WIP status, as I think 7.3.2 is stable on production.

@ibuziuk
Copy link
Member

ibuziuk commented Nov 26, 2019

@amisevsk it looks like that 7.4.0 is affected by eclipse-che/che#15006
I would propose to use this PR for 7.5.0 update (upstream release is planned this week)

@centos-ci
Copy link
Collaborator

@Katka92
Copy link
Collaborator

Katka92 commented Nov 29, 2019

[test]

@centos-ci
Copy link
Collaborator

@centos-ci
Copy link
Collaborator

amisevsk and others added 8 commits December 3, 2019 16:16
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Pull upstream change 1c5c92147f - disable autosave on devfile editor
Pull upstream change 2181ed1340 - fix dashboard build

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Changes upstream mean that the create-workspace controller no longer
hosts devfile selection. Changes required to apply ephemeral mode by
default are instead moved to the ready-to-go-stacks controller.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Commit 51bb0bb13f in upstream causes builds to fail on any webpack
error. To work around a typescript issue, commit 2181ed1340 adds
'index.d.ts' to the dashboard base directory. Since this is not included
in the packaged maven artifact, this commit adds the same file to the
rh-che dashboard directory and updates the build to copy it along with
the rest of the sources.

Should be undone when typescript is updated >3.5

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Upstream commit 32515517bf removes che-ide-fetcher.service.ts and the
cheBranding functions it depends on. This causes a webpack error on
build and makes the dashboard unbuildable.

Until a solution is found, this commit comments out
che-ide-fetcher.service.ts

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Upstream commit 11bbcf0c10 reworks how filtering works in Che, removing
AbstractKeycloakFilter. As the Fabric8WsMasterModule depends on this
filter, and uses it to disable authentication on some paths, this needs
to be removed from rh-che.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@amisevsk
Copy link
Collaborator Author

amisevsk commented Dec 3, 2019

I have added 5 commits to let us move to 7.5.1:

  • 6061e23 : A basic sync of changed upstream dashboard files. PR [UD] Add ability to create a new workspace from the dashboard with a devfile eclipse-che/che#15143 adds the ability to specify a devfile directly, and as a result, the create-workspace page has been updated. This commit moves the ephemeral mode toggle (and setting ephemeral mode = true by default) to the new ready-to-go-stacks controller.
  • c289334 : fixes the dashboard build in general:
  • 083102a : comments out che-ide-fetcher.service.ts. As a result of the above webpack change, a long standing (?) issue is revealed: PR eclipse-che/che@32515517bf removed che-ide-fetcher.service.ts from the repo, and also removed method getIdeResourcesPath from CheBranding. This occurred for 7.3.0 as far as I can tell, but since webpack errors didn't fail the build previously, our class calling the nonexistent method did not seem to raise errors.
    • The specifically failing line is here, though it's not clear how to use this class given that it's removed upstream.
  • 4473003 : the usual "update our classes to accomodate changes to upstream APIs" -- mostly just changing constructor signatures
  • 7b9cb76 : remove the DisableAuthenticationInterceptor from rh-che, as PR Use cached sessions behind any of the login filters eclipse-che/che#15000 removes AbstractKeycloakFilter. I'm not sure this is the correct thing to do here, but as far as I can tell, the interceptor is no longer necessary.

The changes I think are important to double check are:

  1. Does removing che-ide-fetcher.service.ts from the repo cause problems, and what are our options in bringing it back?
  2. Ensure there are no issues around DisableAuthenticationInterceptor, as the upstream PR is quite complex so it's hard to say for sure what additional changes may be necessary.

@amisevsk
Copy link
Collaborator Author

amisevsk commented Dec 3, 2019

Looks like there are some issues with commit 7b9cb76, as I'm seeing a "Authorization token is missing" error when trying to access the dashboard.

@centos-ci
Copy link
Collaborator

@ibuziuk
Copy link
Member

ibuziuk commented Dec 4, 2019

@amisevsk I have provided a PR based on your commits for 7.5.1 update - #1722 (PR check seem to be passing). Are you ok to close this one in favour of #1722 ?

@amisevsk
Copy link
Collaborator Author

amisevsk commented Dec 4, 2019

Closing in favor of #1722, thanks for fixing @ibuziuk

@amisevsk amisevsk closed this Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants