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

[Python][CI] Compatibility with newer minio #37692

Closed
h-vetinari opened this issue Sep 13, 2023 · 8 comments
Closed

[Python][CI] Compatibility with newer minio #37692

h-vetinari opened this issue Sep 13, 2023 · 8 comments

Comments

@h-vetinari
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

Conda-forge very recently added minio, which means we could now use that, at least in the conda(-forge) jobs. I'd be interested in doing that because I see a bunch of skips in the test suite of the following kind (not exhaustive):

SKIPPED [2] pyarrow/tests/test_dataset.py:2623: `minio` command cannot be located
SKIPPED [1] pyarrow/tests/test_dataset.py:2638: `minio` command cannot be located
SKIPPED [1] pyarrow/tests/test_dataset.py:2664: `minio` command cannot be located
SKIPPED [1] pyarrow/tests/test_dataset.py:4754: `minio` command cannot be located
SKIPPED [1] pyarrow/tests/test_dataset.py:4821: `minio` command cannot be located
SKIPPED [1] pyarrow/tests/test_fs.py:471: `minio` command cannot be located
SKIPPED [2] pyarrow/tests/test_fs.py:562: `minio` command cannot be located

so if all it takes is adding the test dependency - why not?

As it turns out, this blows up pretty badly on main (through a proxy PR on our infra), which is at least partly, because arrow currently pins to a relatively outdated version

# Use specific versions for minio server and client to avoid CI failures on new releases.
minio_version="minio.RELEASE.2022-05-26T05-48-41Z"
mc_version="mc.RELEASE.2022-05-09T04-08-26Z"

while the oldest available version in conda-forge is 2023.08.23.10.07.06.

Aside from 2-3 exceptions, the failures are all from the teardown of the s3fs and py_fsspec_s3fs fixtures, where the code unconditionally does fs.delete_dir(bucket), even though the test presumably scribbled stuff in there. This leads to errors of the kind:

_ ERROR at teardown of test_filesystem_is_functional_after_pickling[builtin_pickle-S3FileSystem] _

request = <SubRequest 's3fs' for <Function test_filesystem_is_functional_after_pickling[builtin_pickle-S3FileSystem]>>
s3_server = {'connection': ('localhost', 41707, 'arrow', 'apachearrow'), 'process': <Popen: returncode: None args: ['minio', '--compat', 'server', '--quiet', '-...>, 'tempdir': local('/tmp/pytest-of-conda/pytest-0')}

    @pytest.fixture
    def s3fs(request, s3_server):
        [...]
>       fs.delete_dir(bucket)

pyarrow/tests/test_fs.py:258: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pyarrow/_fs.pyx:616: in pyarrow._fs.FileSystem.delete_dir
    check_status(self.fs.DeleteDir(directory))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   raise convert_status(status)
E   OSError: When deleting bucket 'pyarrow-filesystem': AWS Error UNKNOWN (HTTP status 409) during DeleteBucket operation: Unable to parse ExceptionName: BucketNotEmpty Message: The bucket you tried to delete is not empty

pyarrow/error.pxi:91: OSError

more specifically, close to ~60 of them:

= 4 failed, 7276 passed, 223 skipped, 19 deselected, 28 xfailed, 2 xpassed, 8 warnings, 57 errors in 177.84s (0:02:57) =

My first guess would be that:

  • either minio got stricter (less buggier) in when it allows deletion of non-empty buckets
  • or something in the conda-forge setup does not yet accurately reproduce what the CI is doing here.

I'm quite out of my depths here, but I think a alternative would be to somehow pipe through ForceBucketDelete or --force. A cheap alternative is the following patch (also doesn't cover 100%), which just doesn't care about failed bucket deletes:

--- a/python/pyarrow/tests/test_fs.py
+++ b/python/pyarrow/tests/test_fs.py
@@ -256,7 +256,10 @@ def s3fs(request, s3_server):
         allow_move_dir=False,
         allow_append_to_file=False,
     )
-    fs.delete_dir(bucket)
+    try:
+        fs.delete_dir(bucket)
+    except OSError:
+        pass


 @pytest.fixture
@@ -358,7 +361,10 @@ def py_fsspec_s3fs(request, s3_server):
         allow_move_dir=False,
         allow_append_to_file=True,
     )
-    fs.delete_dir(bucket)
+    try:
+        fs.delete_dir(bucket)
+    except OSError:
+        pass


 @pytest.fixture(params=[

After that patch, the only remaining errors are then:

FAILED pyarrow/tests/test_fs.py::test_get_file_info[PyFileSystem(FSSpecHandler(s3fs.S3FileSystem()))] - AssertionError: assert <FileType.Directory: 3> == <FileType.NotFound: 0>
 +  where <FileType.Directory: 3> = <FileInfo for 'pyarrow-filesystem/a/aa/aaa/': type=FileType.Directory>.type
 +  and   <FileType.NotFound: 0> = FileType.NotFound
FAILED pyarrow/tests/test_fs.py::test_delete_dir[S3FileSystem] - Failed: DID NOT RAISE <class 'OSError'>
FAILED pyarrow/tests/test_fs.py::test_delete_dir_contents[S3FileSystem] - Failed: DID NOT RAISE <class 'OSError'>
FAILED pyarrow/tests/test_fs.py::test_move_directory[PyFileSystem(FSSpecHandler(s3fs.S3FileSystem()))] - Failed: DID NOT RAISE <class 'OSError'>

This issue does not happen on 13.0.0?!

The final kicker is that all this is passing with arrow 13 - I even checked that the tests didn't get skipped. So it appears there are at least two things at work here: a change in minio behaviour & a change in pyarrow somewhere.

Component(s)

Continuous Integration, Python

@kou kou changed the title Compatibility with newer minio [Python][CI] Compatibility with newer minio Sep 13, 2023
@pitrou
Copy link
Member

pitrou commented Sep 21, 2023

@h-vetinari Can you try running the Arrow C++ tests with this new Minio version and report the results?

@h-vetinari
Copy link
Contributor Author

h-vetinari commented Sep 21, 2023

I don't understand - that's exactly what I did in conda-forge/arrow-cpp-feedstock#1170. You can look at one of the resulting CI runs here (but it contains exactly what I documented in the OP).

@pitrou
Copy link
Member

pitrou commented Sep 21, 2023

Ok, but if you don't report the results on the Arrow issue tracker, then nobody notices :-)

@pitrou
Copy link
Member

pitrou commented Sep 21, 2023

Wait, no, the CI run you're pointing to is showing the PyArrow tests. I'm talking about the Arrow C++ tests.

@h-vetinari
Copy link
Contributor Author

I'm talking about the Arrow C++ tests.

Ah, sorry for the misunderstanding. I've been trying to enable the C++ tests in conda-forge/arrow-cpp-feedstock#1058, but it's not passing yet (completely aside from this issue here).

Ok, but if you don't report the results on the Arrow issue tracker, then nobody notices :-)

I had opened #35587 for that. 🙃

@amoeba
Copy link
Member

amoeba commented Sep 24, 2024

@pitrou in case it's helpful, I ran the test suite from a recent C++ build with minio RELEASE.2024-09-09T16-59-28Z on macOS aarch64 and got a few failures. Here's one as an example:

[ RUN      ] TestS3FS.GetFileInfoSelectorRecursive
/Users/bryce/src/apache/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:826: Failure
Failed
'_error_or_value66.status()' failed with IOError: When listing objects under key '' in bucket 'empty-bucket': AWS Error UNKNOWN (HTTP status 429) during ListObjectsV2 operation: Unable to parse ExceptionName: TooManyRequests Message: Please reduce your request rate

Key part there being "Please reduce your request rate".

I then set export MINIO_API_REQUESTS_MAX=1600 and then only got two failures which should be easily resolved. It looks like some behavior changed in minio:

[ RUN      ] TestS3FS.CreateDir
/Users/bryce/src/apache/arrow/cpp/src/arrow/filesystem/s3fs_test.cc:1011: Failure
Failed
Expected 'fs_->CreateDir("bucket/somefile")' to fail with IOError, but got OK

@pitrou
Copy link
Member

pitrou commented Sep 24, 2024

@amoeba Thanks for the report. It should be useful when someone tries to update Minio on CI, I guess.

@h-vetinari
Copy link
Contributor Author

For the purposes of testing the python-bits, this issue has been solved in 18.0.0rc0 (presumably by #44225) - thank you! I've been waiting for arrow v18 to enable testing the C++ library as well, will report back in #35587.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants