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

ensure a space is added if both args are set in ImageSpec #2806

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

blaketastic2
Copy link
Contributor

Why are the changes needed?

If both --index-url and --extra-index-url are set, a space is missing between the 2, resulting in an error.

What changes were proposed in this pull request?

How was this patch tested?

Added unit test and visualized the docker command output.

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Signed-off-by: Blake Jackson <blake@BB8.local>
Copy link

welcome bot commented Oct 11, 2024

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

flytekit/image_spec/default_builder.py Outdated Show resolved Hide resolved
Signed-off-by: Blake Jackson <blake@BB8.local>
@blaketastic2 blaketastic2 force-pushed the support-multiple-values branch from 4815139 to 40bea53 Compare October 11, 2024 14:38
@thomasjpfan thomasjpfan changed the title ensure a space is added if both args are set ensure a space is added if both args are set in ImageSpec Oct 11, 2024
@thomasjpfan thomasjpfan merged commit 2240062 into flyteorg:master Oct 11, 2024
104 checks passed
Copy link

welcome bot commented Oct 11, 2024

Congrats on merging your first pull request! 🎉

otarabai pushed a commit to otarabai/flytekit that referenced this pull request Oct 15, 2024
)

* ensure a space is added if both args are set

Signed-off-by: Blake Jackson <blake@BB8.local>

* removed space normalizing step

Signed-off-by: Blake Jackson <blake@BB8.local>

---------

Signed-off-by: Blake Jackson <blake@BB8.local>
Co-authored-by: Blake Jackson <blake@BB8.local>
@blaketastic2 blaketastic2 deleted the support-multiple-values branch October 29, 2024 12:27
kumare3 pushed a commit that referenced this pull request Nov 8, 2024
* ensure a space is added if both args are set

Signed-off-by: Blake Jackson <blake@BB8.local>

* removed space normalizing step

Signed-off-by: Blake Jackson <blake@BB8.local>

---------

Signed-off-by: Blake Jackson <blake@BB8.local>
Co-authored-by: Blake Jackson <blake@BB8.local>
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