Skip to content

Changes due to IFN upgrade#830

Open
ddelpiano wants to merge 31 commits intodevelopfrom
IFNS-29-upgrade-ifn-2
Open

Changes due to IFN upgrade#830
ddelpiano wants to merge 31 commits intodevelopfrom
IFNS-29-upgrade-ifn-2

Conversation

@ddelpiano
Copy link
Copy Markdown
Member

No description provided.

zoran-sinnema and others added 30 commits September 5, 2025 16:20
This reverts commit d430b3d, reversing
changes made to 6ad98d1.
@ddelpiano ddelpiano requested review from Copilot and zsinnema January 21, 2026 11:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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=True parameter to get_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:
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@zsinnema zsinnema left a comment

Choose a reason for hiding this comment

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

@ddelpiano I think this should go into the development branch of CH, wdyt?

@ddelpiano ddelpiano changed the base branch from IFNS-29-upgrade-ifn to develop April 13, 2026 19:59
@ddelpiano ddelpiano changed the title some fixes to the localIp for values.yaml Changes due to IFN upgrade Apr 13, 2026
@ddelpiano
Copy link
Copy Markdown
Member Author

@zsinnema @filippomc what would you like me to do with this branch? I can see there are some changes that are reverting to older versions we currently have in develop, not sure this has to into develop or we want to bring it first in a release that is not that far from the upgrades that Zoran started and then merge all back to develop, let me know what you prefer. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants