From 5c3fa229c9b10a5a47b93572b4ff6cbd883c7052 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Tue, 4 Jun 2024 16:26:30 -0700 Subject: [PATCH] docs: clarify purpose and implementation of botocore patch testing Signed-off-by: Tony Chen --- python/Makefile | 13 ++++--- python/aistore/botocore_patch/README.md | 11 ++++-- python/aistore/sdk/bucket.py | 2 +- python/aistore/sdk/job.py | 4 +- python/tests/README.md | 2 +- python/tests/botocore_common.py | 17 +++------ python/tests/unit/botocore_patch/README.md | 37 ++++++++++++------- .../unit/botocore_patch/mock_s3_redirect.py | 10 ++--- 8 files changed, 52 insertions(+), 44 deletions(-) diff --git a/python/Makefile b/python/Makefile index 8947df79c6..4e14e6fe29 100644 --- a/python/Makefile +++ b/python/Makefile @@ -1,6 +1,7 @@ SHELL := /bin/bash PYTHON = python3 +PYTEST = $(PYTHON) -m pytest PIP = pip3 DOCFILE := ../docs/python_sdk.md PYAISLOADER_TEST_TYPE ?= short @@ -61,15 +62,15 @@ python_sdk_tests: common_deps python_sdk_unit_tests python_sdk_integration_tests .PHONY: python_sdk_integration_tests python_sdk_integration_tests: common_deps - pytest -v tests/integration/sdk -m "not etl" + $(PYTEST) -v tests/integration/sdk -m "not etl" .PHONY: python_sdk_unit_tests python_sdk_unit_tests: common_deps - pytest -v tests/unit/sdk + $(PYTEST) -v tests/unit/sdk .PHONY: python_etl_tests python_etl_tests: common_deps - pytest -v -s tests/integration/sdk/ -m etl + $(PYTEST) -v -s tests/integration/sdk/ -m etl .PHONY: python_s3_compat_test python_s3_compat_test: s3_test_deps @@ -81,16 +82,16 @@ python_botocore_tests: common_deps botocore_deps python_botocore_unit_tests pyth .PHONY: python_botocore_unit_tests python_botocore_unit_tests: common_deps botocore_deps - pytest -v -n $(BOTO_UNIT_TEST_COUNT) --dist loadfile tests/unit/botocore_patch + $(PYTEST) -v -n $(BOTO_UNIT_TEST_COUNT) --dist loadfile tests/unit/botocore_patch .PHONY: python_botocore_integration_tests python_botocore_integration_tests: common_deps botocore_deps - pytest -v tests/integration/botocore_patch tests/integration/boto3 + $(PYTEST) -v tests/integration/botocore_patch tests/integration/boto3 # Tests for aistore.pytorch .PHONY: python_pytorch_unit_tests python_pytorch_unit_tests: common_deps dev_deps - pytest -v tests/unit/pytorch + $(PYTEST) -v tests/unit/pytorch .PHONY: lint lint: common_deps diff --git a/python/aistore/botocore_patch/README.md b/python/aistore/botocore_patch/README.md index 3daceeba25..4ff8463f0c 100644 --- a/python/aistore/botocore_patch/README.md +++ b/python/aistore/botocore_patch/README.md @@ -1,10 +1,15 @@ -## AIS Botocore Patch -As an alternative to the [AIStore Python SDK](https://aiatscale.org/docs/python_sdk.md), you may prefer to use Amazon's popular [Boto3](https://github.com/boto/boto3) library, or possibly [botocore](https://github.com/boto/botocore), which *boto3* uses under the hood. +# AIS Botocore Patch + +As an alternative to the [AIStore Python SDK](https://aiatscale.org/docs/python_sdk.md) for accessing AIStore, you might prefer using other popular object storage client libraries. For example, you can use Amazon's [Boto3](https://github.com/boto/boto3) library, or its underlying [botocore](https://github.com/boto/botocore) library. + +This package `aistore.botocore_patch.botocore` exposes an interface to access AIStore as if it were Amazon S3, allowing developers to utilize AIStore object storage without changing their existing S3 client code. + +## Install and Import AIStore `botocore_patch` Package By default, botocore doesn't handle [HTTP redirects](https://www.rfc-editor.org/rfc/rfc7231#page-54), which prevents you from using it with AIStore. -To resolve this, install `aistore` with the `botocore` extra, and then import `aistore.botocore_patch.botocore` in your code. This will dynamically patch HTTP redirect support into botocore, via [monkey patch](https://www.tutorialspoint.com/explain-monkey-patching-in-python). +To resolve this, install `aistore` with the `botocore` extra, and then import `aistore.botocore_patch.botocore` in your code. This will dynamically patch HTTP redirect support into botocore, via [monkey patch](https://www.tutorialspoint.com/explain-monkey-patching-in-python). ```shell $ pip install aistore[botocore] diff --git a/python/aistore/sdk/bucket.py b/python/aistore/sdk/bucket.py index 43f5b277dd..f15e60327b 100644 --- a/python/aistore/sdk/bucket.py +++ b/python/aistore/sdk/bucket.py @@ -388,7 +388,7 @@ def info( aistore.sdk.errors.AISError: All other types of errors with AIStore """ try: - FLTPresence(flt_presence) + flt_presence = int(FLTPresence(flt_presence)) except Exception as err: raise ValueError( "`flt_presence` must be in values of enum FLTPresence" diff --git a/python/aistore/sdk/job.py b/python/aistore/sdk/job.py index b405dfd1d3..a85dc63c2d 100644 --- a/python/aistore/sdk/job.py +++ b/python/aistore/sdk/job.py @@ -263,10 +263,10 @@ def get_within_timeframe( for snapshot in snapshots: if snapshot.id == self.job_id or snapshot.kind == self.job_kind: snapshot_start_time = datetime.fromisoformat( - snapshot.start_time.rstrip("Z") + snapshot.start_time[:26] ).time() snapshot_end_time = datetime.fromisoformat( - snapshot.end_time.rstrip("Z") + snapshot.end_time[:26] ).time() if snapshot_start_time >= start_time and snapshot_end_time <= end_time: jobs_found.append(snapshot) diff --git a/python/tests/README.md b/python/tests/README.md index daeb9f6eb4..6a4310d6c9 100644 --- a/python/tests/README.md +++ b/python/tests/README.md @@ -1,6 +1,6 @@ # Python Tests -This directory contains unit tests and integration tests for each of the python packages we provide. +This directory contains unit tests and integration tests for each of the python package interfaces we provide to access AIStore, including the Amazon S3 botocore, SDK, and Pytorch datasets APIs. It also contains tests for verifying s3 compatibility. --- diff --git a/python/tests/botocore_common.py b/python/tests/botocore_common.py index bef1537886..dffc3a1f31 100644 --- a/python/tests/botocore_common.py +++ b/python/tests/botocore_common.py @@ -208,24 +208,17 @@ def __init__(self, redirect_errors_expected=False, operation=None): def __enter__(self): return self - def __exit__(self, exc, value, traceback): + def __exit__(self, exc_type, exc_value, traceback): if self.redirect_errors_expected: - try: - if exc and value: - raise value - except ClientError as ex: + if exc_type is not None and isinstance(exc_value, ClientError): # Some operations don't pass through redirect errors directly if self.operation in ["_bucket_response_put"]: return True - if int(ex.response["Error"]["Code"]) in [302, 307]: + if int(exc_value.response["Error"]["Code"]) in [302, 307]: return True - instead = "No error" - if value: - instead = value raise Exception( - "A ClientError with a redirect code was expected, " - + "but didn't happen. Instead: " - + instead + "A ClientError with a redirect code was expected, but didn't happen. " + f"Instead: {exc_value if exc_value else 'No error'}" ) return False diff --git a/python/tests/unit/botocore_patch/README.md b/python/tests/unit/botocore_patch/README.md index eba755025e..9c48e82c1b 100644 --- a/python/tests/unit/botocore_patch/README.md +++ b/python/tests/unit/botocore_patch/README.md @@ -1,22 +1,30 @@ -# A note on botocore monkey patch testing +# Botocore Monkey Patch Testing -These tests verify the behavior of aistore.botocore_patch.botocore, a monkey patch -for the Amazon botocore library that modifies it to respect the HTTP -`location` header when receiving a HTTP 301, 302 or 307. +These tests verify the behavior of [`aistore.botocore_patch.botocore`](/python/aistore/botocore_patch/README.md), a monkey patch for the Amazon botocore library that allows developers to access AIStore using popular Amazon [Boto3](https://githbu.com/boto/boto3)'s S3 client code. + +Specifically, these tests verify the Boto3, monkey patched by `aistore.botocore_patch.botocore`, works correctly as configured to respect the HTTP `location` header when receiving a HTTP 301, 302 or 307. + +## Test Sets + +The [`botocore_common.py`](python/tests/botocore_common.py) file contains a common test group to verify a basic set of S3 operations. There are two degrees of freedom to test here: - - We've either monkey patched botocore, or we haven't ("patched" or "unpatched") - - The upstream service either sends redirects (like aistore), or it doesn't ("redirects" or "noredirects"). + - Whether botocore has been monkey-patched ("patched") or not ("unpatched"). + - Whether the upstream service (like AIStore) sends redirects ("redirects") or doesn't ("noredirects"). -We have four test sets which each run the same base fixture: +To fully cover all possibilities, we have four test sets, each inheriting and testing against the same base fixture `botocore_common.py`: - - `test_botocore_noredirect_unpatched.py`: A control case for the default botocore behavior - - `test_botocore_noredirect_patched.py`: A passthrough test to check the monkey patch doesn't break botocores normal behavior - - `test_botocore_redirects_unpatched.py`: Another control test: unpatched botocore should throw errors when redirects happen - - `test_botocore_redirects_patched.py`: Positive tests for the monkey patch itself +| | **No Redirects** | **Redirects** | +|-------------------|----------------------------------------|---------------------------------------| +| **Unpatched** | `test_botocore_noredirect_unpatched.py`: A control case for the default botocore behavior. | `test_botocore_redirects_unpatched.py`: Another control test where unpatched botocore should throw errors when redirects occur. | +| **Patched** | `test_botocore_noredirect_patched.py`: A passthrough test to ensure the monkey patch doesn't break botocore's normal behavior. | `test_botocore_redirects_patched.py`: Positive tests for the monkey patch itself. | -We use [moto](https://github.com/spulec/moto) to mock an S3 like endpoint, and monkey patch it to send redirects when wanted. +## Unit Testing Mocks + +For the purpose of unit testing within the scope of an interface provided by `aistore.botocore_patch.botocore`, here we do not actually set up AIStore as an object storage to test against. Instead, we use [moto](https://github.com/spulec/moto) to mock an S3-like interface, simulating AIStore. Additionally, we implement another monkey patch [`mock_s3_redirect.py`](python/tests/unit/botocore_patch/mock_s3_redirect.py) under moto to send redirects when needed, mimicking the behavior of AIStore. + +## Unit Testing Execution Since both the monkey patch and our moto patches rely on runtime imports, running tests like this: @@ -24,10 +32,10 @@ Since both the monkey patch and our moto patches rely on runtime imports, runnin pytest -v tests/unit ``` -...won't work. It's hard to "unimport" a module in python, and so previous tests will contaminate each other's state. +...won't work. Because it's hard to "unimport" a module in python, previous tests will contaminate each other's state. To run these tests, you need to start a new forked process each time, scoped per test file. -To do this inline we use the pytest-xdist plugin like so: +To achieve this inline we use the pytest-xdist plugin like so: ``` pytest -v -n $(MP_TESTCOUNT) --dist loadfile tests/unit/botocore_patch @@ -36,6 +44,7 @@ pytest -v -n $(MP_TESTCOUNT) --dist loadfile tests/unit/botocore_patch ...which is one of the reasons why this test set is kept separate from those for the aistore SDK proper. ## Testing different boto3 and botocore versions + By default we pull in the latest `boto3` and `botocore` to test against (see `botocore_requirements.dev.txt`). You can alter this behavior by exporting `BOTO3_VERSION` and / or `BOTOCORE_VERSION` prior to running tests: diff --git a/python/tests/unit/botocore_patch/mock_s3_redirect.py b/python/tests/unit/botocore_patch/mock_s3_redirect.py index 168b86f41d..67f724f119 100644 --- a/python/tests/unit/botocore_patch/mock_s3_redirect.py +++ b/python/tests/unit/botocore_patch/mock_s3_redirect.py @@ -6,11 +6,11 @@ import logging import wrapt -# Patch moto - an S3 stubbing library - to -# issue redirects for us. - -# When running tests, fake HTTP redirects -# for the following. +# Patch moto - an S3 mocking library to simulate HTTP redirects, +# which might happen on AIStore but the moto library don't. +# This enhances moto to emulate AIStore behavior more comprehensively, +# Specifically, if the `redirections_enabled` is set to `True` by user, +# fake HTTP redirects for the following operations. # # Not all response operatons found in moto's # S3Response are found below; they don't all