[Storage] Fix exclusion for AWS bucket upload #5128
Conversation
|
/smoke-test -k test_skyignore_exclusions --aws |
|
/smoke-test -k test_skyignore_exclusions --gcp |
|
/smoke-test -k test_skyignore_exclusions --azure |
sky/data/storage.py
Outdated
| os.path.join(src_dir_path, excluded_path.rstrip('/')))): | ||
| # Remove any trailing slash and add '/**' to exclude all | ||
| # contents | ||
| processed_excludes.append(f'{excluded_path.rstrip("/")}/**') |
There was a problem hiding this comment.
not a blocker - do we need /** or is /* sufficient? see
skypilot/sky/data/storage_utils.py
Lines 213 to 216 in bdc880a
There was a problem hiding this comment.
Good catch! This part should be general. Just submitted a commit for this. Also, tested that single '*' works.
There was a problem hiding this comment.
Actually, adding patterns could cause the handling of our zip upload to fail as we don't use pattern matching in the underlying code. I am removing the pattern matching globally. Running the smoke tests for storage again.
|
/smoke-test -k storage --aws |
|
/smoke-test -k test_skyignore_exclusions --gcp |
|
/smoke-test -k test_skyignore_exclusions --azure |
|
/smoke-test --aws |
|
/smoke-test -k test_ignore_exclusions --azure |
|
/smoke-test -k test_ignore_exclusions --gcp |
|
All tests above passed, except for an unrelated test: test_skyserve_update https://buildkite.com/skypilot-1/smoke-tests/builds/604#01961289-9327-40a8-ac2a-bf117f985fd9 |
|
@cg505 There are a few more tests added and some additional changes to the core logic. PTAL |
| List[str] containing files and folders to be ignored. There won't be any | ||
| patterns. |
There was a problem hiding this comment.
@cg505 Could you help double check this?
|
/smoke-test --aws |
|
/smoke-test -k test_ignore_exclusions --azure |
|
/smoke-test -k test_ignore_exclusions --gcp |
| List[str] containing files and folders to be ignored. There won't be any | ||
| patterns. |
|
also seems there are some failing unit tests |
|
@cg505 PTAL |
|
/smoke-test |
In AWS S3 upload,
--excludeflag requires a tailing**to exclude a folder. This PR fixes this.To reproduce, run the smoke test added in this PR on master:
pytest tests/test_smoke.py -k test_skyignore_exclusions --awsTested (run the relevant ones):
bash format.sh/smoke-test(CI) orpytest tests/test_smoke.py(local)/smoke-test -k test_name(CI) orpytest tests/test_smoke.py::test_name(local)/quicktest-core(CI) orpytest tests/smoke_tests/test_backward_compat.py(local)