bolkedebruin commented on code in PR #36410:
URL: https://github.com/apache/airflow/pull/36410#discussion_r1436064768
##########
airflow/io/path.py:
##########
@@ -173,10 +176,14 @@ def bucket(self) -> str:
@property
def key(self) -> str:
if self._url:
- return self._url.path
+ return self._url.path.lstrip(self.sep)
Review Comment:
It depends how we want the key to an object to look look like
What will this do:
```
s3://my-bucket/my/key
bucket = my-bucket # notice no '/'
key = "my/key"
# or
key = "/my/key"
```
The S3 hook
(https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/_modules/airflow/providers/amazon/aws/hooks/s3.html#S3Hook.get_s3_bucket_key)
is explicit about making it a relative path (so without the initial '/') and
thus generates the first version.
I agree with adding a rstrip or then basically a full `strip`.
--
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]
