S3 URI as a file name attribute

[matez0]

Hello,
If I open a file with the built-in function, then the name attribute value of the file object will be the same as the first path parameter of the open function.

To be consistent with this behavior, I would expect to set the file name attribute to the S3 URI, if I open an S3 URI,
but it is set to the S3 key ID instead.

What do you think?

[ddelange]

Hey @matez0 👋

The decision is a historical one afaik. Not sure why bucket wasn't included at the time.

At the point of setattr, the original URI is no longer known. So reconstructing the URI at that point is not possible, for instance we support multiple schemes (s3/s3n/s3u/s3a).

Either way, such a change would be a breaking change as this is the public API. From my side there's no strong tendency to add bucket: is there a use-case that this would solve which can't be solved in your own code that uses smart_open?

[matez0]

The URI could be (re-)set as the name attribute in the open_uri caller function where the URI is still present.

Are you sure that it is in the API that the file name shall be the S3 key ID?

When I open a file, I would expect that the name attribute can identify the file in the context where the open is called like in case of the built-in open.

I can make a PR, if this fix would be accepted.

[ddelange]

Are you sure that it is in the API that the file name shall be the S3 key ID?

I'm sure that some people will be relying on the current value of the (public API) name attribute.

I checked out how our different modules set the name attribute: none of them add a scheme to it. for instance http module sets basename, ssh module sets the path on the remote server, gcs we rely on the upstream file object (don't know what/if it sets), azure doesn't set it at all, etc. So we're not treating the name attribute consistently.

I'm not against having the name attribute set to the full URI (except maybe when there are credentials in the URI's query params?) consistently for all modules, accompanied by a major version bump.

Thoughts @mpenkov?