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

[Storage] Uploading list of files to bucket fails #1510

Closed
Michaelvll opened this issue Dec 11, 2022 · 4 comments · Fixed by #1519
Closed

[Storage] Uploading list of files to bucket fails #1510

Michaelvll opened this issue Dec 11, 2022 · 4 comments · Fixed by #1519
Labels
bug Something isn't working
Milestone

Comments

@Michaelvll
Copy link
Collaborator

Seems the following test failed, after this #1494, cc @infwinston:

_________________________________________________________________________________________________ TestStorageWithCredentials.test_list_source[StoreType.S3] __________________________________________________________________________________________________
[gw0] linux -- Python 3.10.6 /home/ubuntu/.conda/envs/sky/bin/python3.1
tests/test_smoke.py:1361: in test_list_source
    tmp_local_list_storage_obj.add_store(store_type)
sky/data/storage.py:658: in add_store
    self._sync_store(store)
sky/data/storage.py:725: in _sync_store
    os.path.join(self.source, '.git')):
../.conda/envs/sky/lib/python3.10/posixpath.py:76: in join
    a = os.fspath(a)
E   TypeError: expected str, bytes or os.PathLike object, not list

I believe the reason is that we did not take the self.source to be a list into account in the following place.

try:
if self.source is not None and os.path.isdir(
os.path.join(self.source, '.git')):
logger.warning(f'\'.git\' directory under \'{self.source}\' '
'is excluded during sync.')
store.upload()

@Michaelvll Michaelvll added this to the Storage milestone Dec 11, 2022
@Michaelvll Michaelvll added the bug Something isn't working label Dec 11, 2022
@infwinston
Copy link
Member

Ah yes this is a problem.. Didn't realize source can be a list. we should also correct the type of self.source to either list or str?

source: str,

@concretevitamin
Copy link
Member

I ran into this while running smoke tests. Could we fix it soon @infwinston?

FAILED tests/test_smoke.py::TestStorageWithCredentials::test_list_source[StoreType.S3] - TypeError: expected str, bytes or os.PathLike object, not list
FAILED tests/test_smoke.py::TestStorageWithCredentials::test_list_source[StoreType.GCS] - TypeError: expected str, bytes or os.PathLike object, not list

@Michaelvll
Copy link
Collaborator Author

I ran into this while running smoke tests. Could we fix it soon @infwinston?

FAILED tests/test_smoke.py::TestStorageWithCredentials::test_list_source[StoreType.S3] - TypeError: expected str, bytes or os.PathLike object, not list
FAILED tests/test_smoke.py::TestStorageWithCredentials::test_list_source[StoreType.GCS] - TypeError: expected str, bytes or os.PathLike object, not list

#1519 is fixing this. It will be merged soon. ;)

@infwinston
Copy link
Member

Yes @Michaelvll's PR include the fix. Romil just approved it, but happy to do additional reviews if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants