Repository: great-expectations/great_expectations
Pull request: [BUGFIX] Fix s3 path suffix bug on windows
Resolves #4022 

Changes proposed in this pull request:
- Updates ``InferredAssetS3DataConnector`` to use a custom file path sanitizer that does not rely on the host OS, but assumes (according to S3 convention) a ``/`` separator.

### Definition of Done

- [x] My code follows the Great Expectations [style guide](https://docs.greatexpectations.io/en/latest/contributing/style_guide.html?highlight=style%20guide)
- [x] I have performed a [self-review](https://docs.greatexpectations.io/en/latest/contributing/contribution_checklist.html?highlight=checklist) of my own code
- [x] I have commented my code, particularly in hard-to-understand areas


Problem statement:
S3 prefixes get broken when running on Windows
**Describe the bug**
On Windows when referencing an S3 data source, the prefix is suffixed with ``\`` instead of ``/``, resulting in query always failing.

**To Reproduce**
Create a ``InferredAssetS3DataConnector`` data connector, and pass any valid prefix (probably needs to be non-empty). Then, attempt to load a file from that connector.

**Expected behavior**
The file gets loaded properly.

**Error**
```
...\lib\site-packages\great_expectations\datasource\data_connector\util.py in list_s3_keys(s3, query_options, iterator_dict, recursive)
    452 
    453     if not any(key in s3_objects_info for key in ["Contents", "CommonPrefixes"]):
--> 454         raise ValueError("S3 query may not have been configured correctly.")
    455 
    456     if "Contents" in s3_objects_info:

ValueError: S3 query may not have been configured correctly.
```

**Fix**
In ``inferred_asset_s3_data_connector.py``, line 82 is:

```
        self._prefix = FilePathDataConnector.sanitize_prefix(prefix)
```

This ensures that the file path is always a folder, by appending a slash to the end. However, on Windows, the slash is ``\``, and thus you end up with a prefix like ``path/to/some/folder\``, which results in an S3 query with no results, hence the above stacktrace.

The fix is to ensure that only the correct slash can be appended to the prefix, regardless of the host OS. For example:

```
        self._prefix = prefix.rstrip('/') + '/'
```

or something to that effect.

**Environment (please complete the following information):**
 - Operating System: Windows
 - Great Expectations Version: 0.14.1

Hints from issue discussion:
Howdy @scnerd, thanks for reaching out. I'm bringing this up with the team, we'll let keep you informed.
@AFineDayFor Working on a PR for this as well.

Commit message: Added docstring

Patch to review:
diff --git a/great_expectations/datasource/data_connector/inferred_asset_s3_data_connector.py b/great_expectations/datasource/data_connector/inferred_asset_s3_data_connector.py
index 0437bead85b8..58db8a4d409f 100644
--- a/great_expectations/datasource/data_connector/inferred_asset_s3_data_connector.py
+++ b/great_expectations/datasource/data_connector/inferred_asset_s3_data_connector.py
@@ -79,7 +79,7 @@ def __init__(
         )
 
         self._bucket = bucket
-        self._prefix = FilePathDataConnector.sanitize_prefix(prefix)
+        self._prefix = self.sanitize_prefix(prefix)
         self._delimiter = delimiter
         self._max_keys = max_keys
 
@@ -93,6 +93,22 @@ def __init__(
                 "Unable to load boto3 (it is required for InferredAssetS3DataConnector)."
             )
 
+    @staticmethod
+    def sanitize_prefix(text: str) -> str:
+        """
+        Takes in a given user-prefix and cleans it to work with file-system traversal methods
+        (i.e. add '/' to the end of a string meant to represent a directory)
+
+        Customized for S3 paths, ignoring the path separator used by the host OS
+        """
+        path_parts = text.split("/")
+        if not path_parts:  # Empty prefix
+            return text
+        elif "." in path_parts[-1]:  # File, not folder
+            return text
+        else:  # Folder, should have trailing /
+            return text.rstrip("/") + "/"
+
     def build_batch_spec(self, batch_definition: BatchDefinition) -> S3BatchSpec:
         """
         Build BatchSpec from batch_definition by calling DataConnector's build_batch_spec function.


Code review comments:
diff_hunk: @@ -93,6 +93,22 @@ def __init__(
                 "Unable to load boto3 (it is required for InferredAssetS3DataConnector)."
             )
 
+    @staticmethod
+    def sanitize_prefix(text: str) -> str:
path: great_expectations/datasource/data_connector/inferred_asset_s3_data_connector.py
text: @user1:
Could we change the name of this to `sanitize_prefix_for_s3` unless this will be used for anything else?

Additionally, would you please add this to the `ConfiguredAssetS3DataConnector`?