some fixes to the localIp for values.yaml#830
some fixes to the localIp for values.yaml#830ddelpiano wants to merge 1 commit intoIFNS-29-upgrade-ifnfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the localIp retrieval in values.yaml for local development by adding LoadBalancer IP detection and ensuring the local=True parameter is passed to get_cluster_ip().
Changes:
- Added LoadBalancer IP detection from ingress-nginx service as the preferred method for local development
- Fixed function call to pass
local=Trueparameter toget_cluster_ip()
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/deployment-cli-tools/ch_cli_tools/utils.py | Added LoadBalancer IP detection logic before minikube IP fallback |
| tools/deployment-cli-tools/ch_cli_tools/helm.py | Corrected function call to pass local=True parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ], timeout=5).decode("utf-8").strip() | ||
| if out and out != '<no value>': | ||
| return out | ||
| except: |
There was a problem hiding this comment.
Bare except clause catches all exceptions including system exits and keyboard interrupts. Specify except Exception: to catch only expected exceptions while allowing system exits to propagate.
zsinnema
left a comment
zsinnema
left a comment
There was a problem hiding this comment.
@ddelpiano I think this should go into the development branch of CH, wdyt?
2 participants
No description provided.