Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Return filesystem properties not user defined metadata in Azure file reads #38330

Closed
Tom-Newton opened this issue Oct 18, 2023 · 0 comments · Fixed by #38524
Closed

[C++] Return filesystem properties not user defined metadata in Azure file reads #38330

Tom-Newton opened this issue Oct 18, 2023 · 0 comments · Fixed by #38524

Comments

@Tom-Newton
Copy link
Contributor

Tom-Newton commented Oct 18, 2023

Describe the enhancement requested

#38269 implements Azure file reads including fetching file metadata. The implementation in that PR chose to just return the user defined metadata on the blob as it was in #12914.

However this is not quite right. I think really we want to return the information in the BlobPropeties which contains more system level information like last modified time, etc. Looking at the GCS filessytem it contains information fairly similar to what is in BlobPropeties

Probably this will require a big function to map all the fields of BlobPropeties to KeyValueMetadata and a corresponding update to the OpenInputStreamReadMetadata test. I thought it would be best to do this in a separate PR in the interest of keeping PRs small and easily re-viewable.

Related Issues:

Component(s)

C++

@Tom-Newton Tom-Newton changed the title Return filesystem properties not user defined metadata in Azure file reads [C++] Return filesystem properties not user defined metadata in Azure file reads Oct 18, 2023
kou added a commit to kou/arrow that referenced this issue Oct 31, 2023
@kou kou closed this as completed in #38524 Nov 7, 2023
kou added a commit that referenced this issue Nov 7, 2023
### Rationale for this change

We use user defined metadata for input stream metadata for now. But we should use properties returned from Azure like other remove filesystem implementations such as S3 and GCS.

### What changes are included in this PR?

Convert `Azure::Storage::Blobs::Models::BlobProperties` to `KeyValueMetadata`. The following values aren't supported yet:

* `BlobProperties::ObjectReplicationSourceProperties`
* `BlobProperties::Metadata`

If they need, we will add support for them as a follow-up task.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: #38330

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
@kou kou added this to the 15.0.0 milestone Nov 7, 2023
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Nov 9, 2023
…apache#38524)

### Rationale for this change

We use user defined metadata for input stream metadata for now. But we should use properties returned from Azure like other remove filesystem implementations such as S3 and GCS.

### What changes are included in this PR?

Convert `Azure::Storage::Blobs::Models::BlobProperties` to `KeyValueMetadata`. The following values aren't supported yet:

* `BlobProperties::ObjectReplicationSourceProperties`
* `BlobProperties::Metadata`

If they need, we will add support for them as a follow-up task.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#38330

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…apache#38524)

### Rationale for this change

We use user defined metadata for input stream metadata for now. But we should use properties returned from Azure like other remove filesystem implementations such as S3 and GCS.

### What changes are included in this PR?

Convert `Azure::Storage::Blobs::Models::BlobProperties` to `KeyValueMetadata`. The following values aren't supported yet:

* `BlobProperties::ObjectReplicationSourceProperties`
* `BlobProperties::Metadata`

If they need, we will add support for them as a follow-up task.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#38330

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…apache#38524)

### Rationale for this change

We use user defined metadata for input stream metadata for now. But we should use properties returned from Azure like other remove filesystem implementations such as S3 and GCS.

### What changes are included in this PR?

Convert `Azure::Storage::Blobs::Models::BlobProperties` to `KeyValueMetadata`. The following values aren't supported yet:

* `BlobProperties::ObjectReplicationSourceProperties`
* `BlobProperties::Metadata`

If they need, we will add support for them as a follow-up task.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#38330

Lead-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants