feat: modify charts and tests for new UI release #2665
Conversation
|
You can access the deployment of this PR at https://ci-renku-2665.dev.renku.ch |
lorenzo-cavazzi
force-pushed
the
000-fix-acceptance-tests
branch
from
c175ee5 to
922516c
Compare
|
@lorenzo-cavazzi can you isolate the fix to the acceptance tests from the ui chart update? Because it seems to me that this PR tries to do both in one. But perhaps I am confused. |
|
Sorry for the mess, this includes the first 3 commits from here #2643 and the others are the fixes -- but it works only with the latest (yet unreleased) UI version. |
|
@lorenzo-cavazzi no problem. I just wanted to make sure it wasn't accidental. You dont have to bother cleaning up. We can just merge this after the other PR you mentioned gets merged. |
lorenzo-cavazzi
changed the title
Co-authored-by: Rok Roškar <[email protected]>
lorenzo-cavazzi
force-pushed
the
000-fix-acceptance-tests
branch
from
81cbab5 to
641d5ac
Compare
| baseUrl: https://{renku_domain} | ||
| gatewayUrl: https://{renku_domain}/api | ||
| client: | ||
| uiserverUrl: https://{renku_domain}/ui-server |
There was a problem hiding this comment.
I don't see this set in the default values in the renku chart - why is it set here? Seems like this default should either be set on the component chart or at least on the renku chart.
There was a problem hiding this comment.
But more importantly, can't this always be built from the information contained in the ui.server.serverData values? Since the two charts are now unified, you shouldn't need to declare this manually.
There was a problem hiding this comment.
Do you mean we should add it (commented) to helm-chart/renku/values.yaml ? Or we should drop it since it's set as a default in the UI values? If it's the latter, then we should probably drop baseUrl and gatewayUrl too since the default is already derived from ui.protocol and .Values.global.renku.domain.
There was a problem hiding this comment.
can't this always be built from the information contained in the ui.server.serverData values?
Yes, we can unify some of the values from client and server but that requires a bit more work since the URLs are built in different ways and it would make sense to unify the logic -- should I make an issue in the UI to take care of that?
There was a problem hiding this comment.
yes please do, there is certainly an opportunity here to reduce redundancy
There was a problem hiding this comment.
drop as much as possible from this template (but make sure it still yields a usable values file) because it's meant as a first exposure for someone deploying renku
There was a problem hiding this comment.
This is the issue in the UI SwissDataScienceCenter/renku-ui#1976
I removed all the values except the welcomePage since they are not strictly needed.
/deploy #persist