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

Create output dir instead of returning error for OCI artifacts #246

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

FrenchBen
Copy link
Contributor

@FrenchBen FrenchBen commented Nov 15, 2023

When a user wants to pull an OCI artifact, and they provide a location as the output, they more than likely want the output to be created if it doesn't exist.
CMD modified:

  • mod pull
  • artifact pull

Instead of checking for its existence, it's better to attempt to create it.

  • If it exists, nothing will happen
  • if it's a file, you'll get an error
  • if it doesn't exist, it will be created

The above allows for a smoother UX, instead of getting a generic error:

$ timoni mod pull oci://ghcr.io/stefanprodan/modules/redis --version 7.2.2 --output ./redis-oci 
12:48PM ERR invalid output path ./redis-oci

After the fix:

$ timoni mod pull oci://ghcr.io/stefanprodan/modules/redis --version 7.2.2 --output ./redis-oci
1:32PM INF extracted: ./redis-oci

Test with existing file:

$ touch redis-oci
$ timoni mod pull oci://ghcr.io/stefanprodan/modules/redis --version 7.2.2 --output ./redis-oci
1:32PM ERR error creating path ./redis-oci: mkdir ./redis-oci: not a directory

@FrenchBen FrenchBen changed the title Create dir instead of returnin error Create dir instead of returning error Nov 15, 2023
@stefanprodan stefanprodan changed the title Create dir instead of returning error Create output dir instead of returning error for OCI artifacts Nov 16, 2023
@stefanprodan stefanprodan added the area/cli CLI related issues and pull requests label Nov 16, 2023
cmd/timoni/artifact_pull.go Outdated Show resolved Hide resolved
cmd/timoni/mod_pull.go Outdated Show resolved Hide resolved
FrenchBen and others added 2 commits November 16, 2023 15:00
Co-authored-by: Stefan Prodan <[email protected]>
Co-authored-by: Stefan Prodan <[email protected]>
Copy link
Owner

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @FrenchBen

@stefanprodan stefanprodan merged commit 60c918d into stefanprodan:main Nov 16, 2023
4 checks passed
@FrenchBen FrenchBen deleted the create-dir branch November 14, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli CLI related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants