dstandish commented on code in PR #28707:
URL: https://github.com/apache/airflow/pull/28707#discussion_r1061022996
##########
tests/providers/amazon/aws/hooks/test_s3.py:
##########
@@ -825,3 +826,122 @@ def test_delete_bucket_tagging_with_no_tags(self):
with pytest.raises(ClientError, match=r".*NoSuchTagSet.*"):
hook.get_bucket_tagging(bucket_name="new_bucket")
+
+
+@patch("airflow.hooks.base.BaseHook.get_connection")
[email protected](
+ "expected",
+ [
+ # full key
+ # no conn - no bucket - full key
+ param(["key_bucket", "key.txt"],
id="unify-no_conn-no_bucket-full_key"),
+ param(["key_bucket", "key.txt"],
id="provide-no_conn-no_bucket-full_key"),
+ # no conn - with bucket - full key
+ param(["kwargs_bucket", "s3://key_bucket/key.txt"],
id="unify-no_conn-with_bucket-full_key"),
+ param(["kwargs_bucket", "s3://key_bucket/key.txt"],
id="provide-no_conn-with_bucket-full_key"),
+ # with conn - no bucket - full key
+ param(["conn_bucket", "s3://key_bucket/key.txt"],
id="provide-with_conn-no_bucket-full_key"),
+ param(["key_bucket", "key.txt"],
id="unify-with_conn-no_bucket-full_key"),
+ # with conn - with bucket - full key
+ param(["kwargs_bucket", "s3://key_bucket/key.txt"],
id="provide-with_conn-with_bucket-full_key"),
+ param(["kwargs_bucket", "s3://key_bucket/key.txt"],
id="unify-with_conn-with_bucket-full_key"),
+ # rel key
+ # no conn - no bucket - rel key
+ param("__fail__", id="unify-no_conn-no_bucket-rel_key"),
+ param("__fail__", id="provide-no_conn-no_bucket-rel_key"),
+ # no conn - with bucket - rel key
+ param(["kwargs_bucket", "key.txt"],
id="unify-no_conn-with_bucket-rel_key"),
+ param(["kwargs_bucket", "key.txt"],
id="provide-no_conn-with_bucket-rel_key"),
+ # with conn - no bucket - rel key
+ param(["conn_bucket", "key.txt"],
id="provide-with_conn-no_bucket-rel_key"),
+ param("__fail__", id="unify-with_conn-no_bucket-rel_key"),
+ # with conn - with bucket - rel key
+ param(["kwargs_bucket", "key.txt"],
id="provide-with_conn-with_bucket-rel_key"),
+ param(["kwargs_bucket", "key.txt"],
id="unify-with_conn-with_bucket-rel_key"),
+ ],
+)
+def test_dec_no_get_connection_call(mock_base, expected, request):
+ tokens = request.node.callspec.id.split("-")
+ assert len(tokens) == 4
+ if "with_conn" in tokens:
+ c = Connection(schema="conn_bucket")
+ else:
+ c = Connection(schema=None)
+ key = "key.txt" if "rel_key" in tokens else "s3://key_bucket/key.txt"
Review Comment:
what @ferruzzi is saying is do this
```python
rel_key = "key.txt"
full_key = f"s3://key_bucket/{rel_key}"
key = rel_key if "rel_key" in tokens else full_key
```
i am happy to do if you feel that stongly about itt
this is just a miniscule case of DRY and ... i'm just saying, the asserts
would fail if you screwed it up
it would come out in the asserts meaning, you would know if you screwed it
up because the asserts would let you know
using the id in this way is very helpful for readability. you are welcome
to try it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
