You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2020/09/17 19:12:47 UTC
[GitHub] [incubator-heron] nicknezis opened a new pull request #3619: Added ability to disable base_url in Helm chart
nicknezis opened a new pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619
This PR adds a new config item to support disabling the `base_url` parameter when installing Heron using the Helm chart.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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 config_value` - that would avoid the new coupled variable currently introduced by the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis closed pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis closed pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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 config_value` - that would avoid the new coupled variable currently introduced by the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-697681510
@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?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] windhamwong commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
windhamwong commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-697721988
https://github.com/Masterminds/sprig/issues/53#issuecomment-327904181
This could be an insight of checking null or "".
Maybe checking if .Values.heron.url is empty or not? (eq nil?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis closed pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis closed pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 edited a comment on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 edited a comment on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 edited a comment on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 edited a comment on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis merged pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis merged pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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 config_value` - that would avoid the new coupled variable currently introduced by the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-697873473
I updated the logic to use more features I learned about [Sprig Functions](http://masterminds.github.io/sprig/).
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`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-695889995
Closing this PR because the desired behavior can be attained by setting `.Values.heron.url` to `/`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-695889995
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] Code0x58 edited a comment on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
Code0x58 edited a comment on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696372987
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
nicknezis commented on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-696329808
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] windhamwong edited a comment on pull request #3619: Added ability to disable base_url in Helm chart
Posted by GitBox <gi...@apache.org>.
windhamwong edited a comment on pull request #3619:
URL: https://github.com/apache/incubator-heron/pull/3619#issuecomment-697721988
https://github.com/Masterminds/sprig/issues/53#issuecomment-327904181
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'`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org