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

plan/run_sh/store: providing a directory path doesn't copy the content of it #2227

Closed
Bidon15 opened this issue Feb 27, 2024 · 4 comments · Fixed by #2326 or #2410
Closed

plan/run_sh/store: providing a directory path doesn't copy the content of it #2227

Bidon15 opened this issue Feb 27, 2024 · 4 comments · Fixed by #2326 or #2410
Assignees
Labels
bug Something isn't working papercut sdk for bugs/FRs relating to our SDK

Comments

@Bidon15
Copy link

Bidon15 commented Feb 27, 2024

What's your CLI version?

0.86.12

Description & steps to reproduce

  1. Try to run an bash plan command
  2. Provide a store path, so the content can be mounted for the plan.add_service part later
results = plan.run_sh(
        # image and others 
        store=[
            "/foo/bar/",
        ],
    )
 plan.print(results.files_artifacts)

Desired behavior

Content under bar is stored in the context for mounting in later stages like plan.add_service()

Note: adding an asterisk to /foo/bar/* fixes this behaviour for now

What is the severity of this bug?

Papercut; this bug is frustrating, but I have a workaround.

What area of the product does this pertain to?

SDK: the Software Development Kit libraries - Typescript, Go, etc.

@Bidon15 Bidon15 added the bug Something isn't working label Feb 27, 2024
@github-actions github-actions bot added papercut sdk for bugs/FRs relating to our SDK labels Feb 27, 2024
@mieubrisse
Copy link
Member

Extra context: @Dartoxian and myself have both hit this exact same issue. It's pretty overwhelming that folks expect a store'd directory to copy the contents, not the directory itself.

IMPLEMENTATION IDEA: we could make the default behaviour "if it's a directory, copy the contents. only if the user adds an explicit / at the end, e.g. /path/to/my/dir/, will we copy the directory itself"

@Bidon15
Copy link
Author

Bidon15 commented Feb 27, 2024

IMPLEMENTATION IDEA: we could make the default behaviour "if it's a directory, copy the contents. only if the user adds an explicit / at the end, e.g. /path/to/my/dir/, will we copy the directory itself"

ngl, pretty much 🅱️ased idea

@leovct
Copy link
Contributor

leovct commented Feb 29, 2024

Extra context: @Dartoxian and myself have both hit this exact same issue. It's pretty overwhelming that folks expect a store'd directory to copy the contents, not the directory itself.

IMPLEMENTATION IDEA: we could make the default behaviour "if it's a directory, copy the contents. only if the user adds an explicit / at the end, e.g. /path/to/my/dir/, will we copy the directory itself"

+1 I also ran into this problem when prototyping with kurtosis :)

@h4ck3rk3y
Copy link
Contributor

h4ck3rk3y commented Mar 25, 2024

This is interesting as upload_files has this behavior but run_sh
doesn't so there is inconsistency; store_service_files has this behavior as they use the same backend

note that the "fix" would be a breaking change will affect ethereum-pacakge at least

@h4ck3rk3y h4ck3rk3y self-assigned this Mar 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 29, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.89.0](0.88.19...0.89.0)
(2024-04-29)


### ⚠ BREAKING CHANGES

* copy contents of directory instead of the directory if store is used
for directories
([#2326](#2326))

### Bug Fixes

* copy contents of directory instead of the directory if store is used
for directories
([#2326](#2326))
([4c776be](4c776be)),
closes [#2227](#2227)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working papercut sdk for bugs/FRs relating to our SDK
Projects
None yet
4 participants