docs: clarify purpose and implementation of botocore patch testing

Signed-off-by: Tony Chen <[email protected]>

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

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]

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"

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)

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.
 
 ---

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

python/tests/unit/botocore_patch/README.md

@@ -1,33 +1,41 @@
-# 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:
 
 ```
 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:

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