Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update Solo to load remote config near entry point #1171

Closed
Tracked by #591
jeromy-cannon opened this issue Jan 15, 2025 · 3 comments · Fixed by #1176
Closed
Tracked by #591

update Solo to load remote config near entry point #1171

jeromy-cannon opened this issue Jan 15, 2025 · 3 comments · Fixed by #1176
Assignees
Labels
Multiple Cluster Support Issues which are only required to enable Solo support for multi-cluster deployments. P1 High priority issue. Required to be completed in the assigned milestone. released

Comments

@jeromy-cannon
Copy link
Contributor

jeromy-cannon commented Jan 15, 2025

Currently the remote config is loaded usually after a commands initialize method. The problem here is that lease create is called at the top of the command and lease needs to know which cluster/context to create the lease on. In the case of multi-cluster, we will have multiple leases and multiple K8 instances (not implemented currently), so to avoid possible issues we need to load the remote config as close to the entry point of Solo as possible.

Solution:
In src/index.ts:main(argv: any) add a property that stores a function, similar to processArguments. This function will load the remote config. Then below the current .middleware(processArguments, false) add another middleware call for the function that will load the remote config.

Certain commands do not need to call K8 nor load the remote config. We should check the argv and not load the remote config in these conditions.

  • if help is called, -h, --help
  • if version is called, -v, --version
  • if it is command init
  • if it is command node and subcommand keys

Then remove all of the existing calls that load the remote config.

@jeromy-cannon jeromy-cannon added P1 High priority issue. Required to be completed in the assigned milestone. Multiple Cluster Support Issues which are only required to enable Solo support for multi-cluster deployments. labels Jan 15, 2025
@Ivo-Yankov
Copy link
Contributor

Not related to this issue, but isn't it better to have --version be a separate command instead of a flag?

@jeromy-cannon
Copy link
Contributor Author

Not related to this issue, but isn't it better to have --version be a separate command instead of a flag?

I think it is directly integrated into yargs. I don't think we added it.

@swirlds-automation
Copy link
Contributor

🎉 This issue has been resolved in version 0.34.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Multiple Cluster Support Issues which are only required to enable Solo support for multi-cluster deployments. P1 High priority issue. Required to be completed in the assigned milestone. released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants