Python: Fix warnings from pytest #6703
Conversation
Fokko
force-pushed
the
fd-fix-warnings
branch
from
11889e0 to
33ac205
Compare
Because TestType contains test in the name, pytest tries to collect it and run tests on it. Setting `__test__ = False` tells PyTest to ignore it Also, s3 emits an error because we clean up buckets that still has content in it, so I've added some code to clean them up after we use the bucket.
Fokko
force-pushed
the
fd-fix-warnings
branch
from
33ac205 to
56c80ee
Compare
Co-authored-by: Ajantha Bhat <ajanthabhat@gmail.com>
python/tests/catalog/test_glue.py
Outdated
| Bucket=BUCKET_NAME, | ||
| ) | ||
| while response["KeyCount"] > 0: | ||
| _s3.delete_objects(Bucket=BUCKET_NAME, Delete={"Objects": [{"Key": obj["Key"]} for obj in response["Contents"]]}) |
There was a problem hiding this comment.
I haven't checked the details of this test, but delete_objects can delete at max 1000 keys for a given bucket. We're probably not creating close to that much here which is why this is passing, but just so the logic is robust we probably want to handle that (if not here, we can do in a separate PR as well)
There was a problem hiding this comment.
I'm also no expert on the subject, but it looks like the max number of key returned is also 1000:
From: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.list_objects_v2
This will iterate until KeyCount is zero
Fokko
changed the title
| @@ -422,6 +422,7 @@ def test_void_transform() -> None: | |||
|
|
|||
| class TestType(IcebergBaseModel): | |||
| __root__: Transform[Any, Any] | |||
| __test__ = False | |||
There was a problem hiding this comment.
Was this fixed in the other PR?
There was a problem hiding this comment.
I think that was a PR to the 0.3.0 branch, so we can create another RC from that branch without having the latest pyarrow changes released along with it.
python/tests/catalog/test_glue.py
Outdated
| @@ -67,7 +67,18 @@ def get_random_databases(n: int) -> Set[str]: | |||
|
|
|||
| @pytest.fixture(name="_bucket_initialize") | |||
| def fixture_s3_bucket(_s3) -> None: # type: ignore | |||
| _s3.create_bucket(Bucket=BUCKET_NAME) | |||
| bucket = _s3.create_bucket(Bucket=BUCKET_NAME) | |||
| yield bucket | |||
There was a problem hiding this comment.
Does this change the return type?
There was a problem hiding this comment.
No, this is a bit of an interesting fixture. It didn't return anything before, and now it just blocks the yield until the test is passed, and then cleans it up.
|
This PR needs a bit of TLC; it looks like the adlfs test also throws some warnings that we need to fix. |
…into fd-fix-warnings
Because TestType contains test in the name, pytest tries to collect it and run tests on it. Setting
__test__ = Falsetells PyTest to ignore itAlso, s3 emits an error because we clean up buckets that still has content in it, so I've added some code to clean them up after we use the bucket.