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

Added ability to disable base_url in Helm chart #3619

Merged
merged 2 commits into from
Sep 23, 2020
Merged

Conversation

nicknezis
Copy link
Contributor

This PR adds a new config item to support disabling the base_url parameter when installing Heron using the Helm chart.

@nicknezis
Copy link
Contributor Author

Closing this PR because the desired behavior can be attained by setting .Values.heron.url to /.

@nicknezis nicknezis closed this Sep 21, 2020
@nicknezis nicknezis reopened this Sep 21, 2020
@nicknezis
Copy link
Contributor Author

I tested setting .Values.heron.url to / and things didn't work. I believe this PR is still needed to use Heron UI with k8s NodePorts.

@Code0x58
Copy link
Contributor

Code0x58 commented Sep 21, 2020

does setting --base_url= work as expected as I feel it should? If so, I'm thinking it would be cleaner to use something better than default when setting the url, and to instead use something like value = config_value if config_value is not None else "" - that would avoid the new coupled variable currently introduced by the PR.

@nicknezis
Copy link
Contributor Author

@Code0x58 The issue I ran in when trying something similar to what you suggested, was that "" and null both resolved to a type that was not a String. So my attempt to add a comparison in the template failed. I'm not sure how to check type and type convert in the Helm template. Any suggestions?

windhamwong
windhamwong previously approved these changes Sep 23, 2020
Copy link
Contributor

@windhamwong windhamwong left a comment

Choose a reason for hiding this comment

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

Looks good with the OR comparison to use the default value.

@windhamwong windhamwong dismissed their stale review September 23, 2020 17:09

need to solve the issue of default value

@windhamwong
Copy link
Contributor

windhamwong commented Sep 23, 2020

Masterminds/sprig#53 (comment)
This could be an insight of checking null or "".
Maybe checking if .Values.heron.url is empty or not? (eq nil?)

Helm syntax has no ability to do code = 'True' if True is True else 'False'.

@nicknezis
Copy link
Contributor Author

I updated the logic to use more features I learned about Sprig Functions.

This latest version will use the single variable. If set to "-" it will use the full k8s proxy URL. I've made this the default. If a user sets the heron.url to any string, it will set that as the value. This includes the empty string case "". The third option that I needed was the ability to disable the base_url parameter. This can now be achieved by setting heron.url: null.

@nicknezis nicknezis merged commit fe1c199 into master Sep 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants