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

fix: drop xargs from postStart script #149

Merged
merged 1 commit into from
Mar 2, 2025
Merged

Conversation

Tehsmash
Copy link
Contributor

@Tehsmash Tehsmash commented Feb 27, 2025

xargs has inconsistant behaviour handling whitespace:

  • When given argument -I {} it no longer splits on spaces properly.
  • When given -d" " to force it to split on spaces it doesn't handle single values properly.

This problem identifies itself when there are multiple create models defined in the values today because xargs tries to put every model name in a single create command.

This commit drops the use of xargs completely from the script and uses the helm templating to create all the commands explictly.

Summary of changes:

Checklist:

  • I updated the artifacthub.io/changes annotation in Chart.yml according to the documentation
  • Optional: I updated in README.md the Helm Values

xargs has inconsistant behaviour handling whitespace:
* When given argument `-I {}` it no longer splits on spaces properly.
* When given `-d" "` to force it to split on spaces it doesn't handle
  single values properly.

This problem identifies itself when there are multiple create models defined in
the values today because xargs tries to put every model name in a single
create command.

This commit drops the use of xargs completely from the script and uses
the helm templating to create all the commands explictly.
@jdetroyes
Copy link
Contributor

Hello @Tehsmash

Thanks you for refactoring and optimizing postscript, indeed this is a better way to handle model lists commands.

@jdetroyes jdetroyes merged commit bb12de3 into otwld:main Mar 2, 2025
1 check passed
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.

2 participants