Skip to content

Add test that s3fs.put of single file in directory is repeatable#666

Closed
ianthomas23 wants to merge 1 commit into
fsspec:mainfrom
ianthomas23:659_test_put_single_repeat
Closed

Add test that s3fs.put of single file in directory is repeatable#666
ianthomas23 wants to merge 1 commit into
fsspec:mainfrom
ianthomas23:659_test_put_single_repeat

Conversation

@ianthomas23
Copy link
Copy Markdown
Contributor

I cannot reproduce issue #659 locally, which is that an s3fs.put of a single file in a directory produces different results the first and second times it is run. So this PR adds an explicit test for this situation, to run it through the full CI suite.

@martindurant
Copy link
Copy Markdown
Member

@tilan7663 , can you produce a test similar to the code here that shows your unexpected behaviour?

@tilan7663
Copy link
Copy Markdown

@martindurant @ianthomas23 Thanks for taking a look. I can cherry-pick the change and see if it yields the same behaviour. The behaviour we noticed prior the upgrade is similar to the following

def test_put_single_repeat_2(s3, tmpdir):
    # See https://github.com/fsspec/s3fs/issues/659
    # Potential different behaviour if s3.put called multiple times with
    # a directory containing a single file
    import uuid

    directory = os.path.join(str(tmpdir), "issue659_dir")
    os.mkdir(directory)
    test_file = os.path.join(directory, "issue659_file")
    with open(test_file, "w") as f:
        f.write("some text")

    session_id = uuid.uuid4().hex
    s3_dir = os.path.join(test_bucket_name, session_id)
    s3_file = os.path.join(test_bucket_name, session_id, "issue659_file")
    s3_dir_not_exist = os.path.join(test_bucket_name, session_id, "issue659_dir")
    s3_file_not_exist = os.path.join(test_bucket_name, session_id, "issue659_dir", "issue659_file")

    assert not s3.exists(s3_path)

    for _ in range(2):
        s3.put(directory, s3_dir, recursive=True)
        assert s3.isdir(s3_dir)
        assert s3.isfile(s3_file)
        assert not s3.exists(s3_dir_not_exist)
        assert not s3.exists(s3_file_not_exist)
        assert s3.cat(s3_file) == b"some text"

@ianthomas23
Copy link
Copy Markdown
Contributor Author

@tilan7663 Thanks for the clarification. My test doesn't accurately represent your problem as I've missed out an extra directory level. I will look into it further.

@ianthomas23
Copy link
Copy Markdown
Contributor Author

I can confirm now that I can reproduce this but the issue is really in fsspec not just s3fs. Issue fsspec/filesystem_spec#1062 has the relevant discussion. We should hold off from doing anything here until that is dealt with.

@ianthomas23
Copy link
Copy Markdown
Contributor Author

I am closing this PR as it has been superseded by #691.

@ianthomas23 ianthomas23 closed this Feb 1, 2023
@ianthomas23 ianthomas23 deleted the 659_test_put_single_repeat branch February 1, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants