Console: chart enhance#234
Conversation
|
@MonkeyCanCode do you mind to rebase and resolve conflicts? |
Checking |
This is completed now. |
dimas-b
left a comment
dimas-b
left a comment
There was a problem hiding this comment.
The PR LGTM overall, thanks, @MonkeyCanCode !
Just one minor comment about LICENSE, CC: @jbonofre :)
|
@cccs-cat001 : WDYT? |
adutra
left a comment
adutra
left a comment
There was a problem hiding this comment.
Thank you for this comprehensive overhaul of the chart!
I left some minor comments.
Are we OK though with the breaking change? I guess yes since there seems to be no "official" release of this chart so far.
| # | ||
|
|
||
| # Artifact Hub repository metadata file | ||
| repositoryID: apache-polaris-console # TODO: Update this with the actual repository ID later |
There was a problem hiding this comment.
Let's create the Artifact Hub repository for this chart. But before that, we need to publish at least one version of the chart somewhere, e.g. https://downloads.apache.org/polaris-tools/console/helm-chart/.
cc @jbonofre
There was a problem hiding this comment.
I will include this in the first console release (on dist).
There was a problem hiding this comment.
I don't have access to do so. @adutra can you help on this?
There was a problem hiding this comment.
@MonkeyCanCode we need first to release the chart. I suggest we leave this as-is for now and revisit it later, after the first release.
There was a problem hiding this comment.
One minor thing, as we hasn't push the docker image for this one, we may need to do so along with first publish of the helm chart.
I think so. May be better to fix those before first release. |
|
Thanks for the review @adutra and @jbonofre . Most of the concerns/feedbacks had been addressed. There are couple of them are intensional as to match to polaris helm chart for easy review. If those are needed, it may be better to do them in a follow up PR. The failed CI is not related to this PR but fixed in #245. |
cccs-cat001
left a comment
cccs-cat001
left a comment
There was a problem hiding this comment.
Seems like a great improvement to me :)
5 participants
This PR polish the helm chart used by Polaris Console by matching to the same layout as well as re-using chart templates from Apache Polaris. I also updated both top level and console project level makefile to reflect those and add them to the CI.
This PR also also change the nginx port as well as k8s service pods from
80to8080. This is done to avoid issue likes #231.