-
Notifications
You must be signed in to change notification settings - Fork 198
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
feat(sdk): support mounting sim.Container to state directories #6295
Conversation
Thanks for opening this pull request! 🎉
|
libs/wingsdk/test/target-sim/my-docker-image.mounted-volume/index.js
Dismissed
Show resolved
Hide resolved
Console preview environment is available at https://wing-console-pr-6295.fly.dev 🚀 Last Updated (UTC) 2024-04-22 17:49 |
BenchmarksComparison to Baseline ⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜
⬜ Within 1.5 standard deviations Benchmarks may vary outside of normal expectations, especially when running in GitHub Actions CI. Results
Last Updated (UTC) 2024-04-22 17:54 |
Thanks for contributing, @Chriscbr! This PR will now be added to the merge queue, or immediately merged if |
Congrats! 🚀 This was released in Wing 0.71.9. |
Normalize the code within the SDK so that the paths to various simulator files are stored as absolute paths. This is causing an issue where the feature added in #6295 only works when running `wing it` and not in `wing test` because `wing test` initializes the simulator with a relative path. ## Checklist - [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [x] Description explains motivation and solution - [ ] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job
If a container mounts a volume to the state directory as described in #6295, it can sometimes result in permission errors when the CLI is cleaning up files after `wing test` because the mounted directory may have different permissions. ``` EACCES: permission denied, scandir 'target/test/mount.test.wsim/.state/c892787d9270cdccc06c34c95f7f9738be92b19134' ``` This error only seems to happen on CI for me. I'm not sure of the cause but I'm guessing that in CI, docker runs in a different (rootless maybe?) mode than on macOS. Unblocks winglang/winglibs#207 ## Checklist - [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [ ] Description explains motivation and solution - [ ] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
Another pass at solving #6284 The approach taken in #6295 where we allow users to bind to the state directory is problematic because docker sometimes changes the permissions of directories on the user's file system after mounting them. This was causing `wing test` to fail (fixed in #6320) and `wing pack` to fail as well. This PR reworks the API so that data can be persisted via named volumes. Specifically, an anonymous docker volume can be specified, and Wing will automatically name it and reuse it across Wing Console runs. The change should fix the previous issues since named volumes are managed by docker. The main downside of this approach is that deleting your `target` directory or the state files in `.state/` will not clean everything up all resources/files created by Wing from your machine, as Docker volumes may still exist. Eventually we can provide some facilities for automatically cleaning these, but we're leaving it intentionally out of scope until this general approach has been validated. ## Checklist - [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [x] Description explains motivation and solution - [ ] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
To support retaining the state of
sim.Container
across separate Wing Console sessions, this PR adds the capability to reference the resource's state directory through theWING_STATE_DIR
environment variable. This will be used by thepostgres
winglib to persist state through a mounted volume:Related to #6284
Checklist
pr/e2e-full
label if this feature requires end-to-end testingBy submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.