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.

Sign up for GitHub

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

Jump to bottom

feat(core): support user defined metadata for oss #4881

Merged
merged 4 commits into from
Jul 14, 2024
Merged

feat(core): support user defined metadata for oss #4881

merged 4 commits into from
Jul 14, 2024

Conversation

meteorgan
Copy link
Contributor

@meteorgan meteorgan commented Jul 11, 2024 •
edited
Loading

part of: #4842

Done:

  • Implement user_metadata for metadata
  • Implement user_metadata for oss

Todo:

  • support blocking operator

@github-actions github-actions bot requested review from dqhl76 and Xuanwo July 11, 2024 14:26
meteorgan
meteorgan commented Jul 11, 2024
View reviewed changes
core/src/types/operator/operator_futures.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function comments in FutureWrite and other related structs are not unified and confusing.
for example:

  1. Set the append mode of op
  2. Set the content type of option
  3. Set the executor for this operation

Is the op, option, operation the same meaning ? shall we unify them ?

Copy link
Member

@Xuanwo Xuanwo Jul 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are the same. We can call them option. We can start a new PR for this.

Xuanwo
Xuanwo reviewed Jul 11, 2024
View reviewed changes
core/src/raw/http_util/header.rs Outdated Show resolved Hide resolved
core/src/services/oss/core.rs Outdated Show resolved Hide resolved
core/src/types/operator/operator_futures.rs Outdated Show resolved Hide resolved
@Xuanwo Xuanwo changed the title feat(user_metadata): support user defined metadata for oss feat(core): support user defined metadata for oss Jul 11, 2024
@meteorgan
Copy link
Contributor Author

part of: #4842

Done:

  • Implement user_metadata for metadata
  • Implement user_metadata for oss

Todo:

  • support blocking operator

Currently, none of the services that support blocking operations require user metadata. So, it seems there’s no need to support user metadata for the blocking operator, right? @Xuanwo

@Xuanwo
Copy link
Member

Xuanwo commented Jul 14, 2024

Currently, none of the services that support blocking operations require user metadata. So, it seems there’s no need to support user metadata for the blocking operator, right? @Xuanwo

I'm fine to split user metadata support of blocking operations into other PRs. Or just leave them not implemented.

Xuanwo
Xuanwo approved these changes Jul 14, 2024
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, thanks!

@Xuanwo Xuanwo marked this pull request as ready for review July 14, 2024 14:23
@Xuanwo Xuanwo merged commit b9f7c21 into apache:main Jul 14, 2024
212 of 219 checks passed
@meteorgan meteorgan deleted the user_metadata branch July 15, 2024 13:30
Lzzzzzt pushed a commit to Lzzzzzt/opendal that referenced this pull request Jul 16, 2024
@meteorgan @Lzzzzzt
feat(core): support user defined metadata for oss (apache#4881)
f872ac3 
* feat(user_metadata): support user defined metadata for oss

* fix cargo fmt && doc tests

* Add user metadata key checks for oss && refeactor some code

* remove unused code
@Xuanwo Xuanwo mentioned this pull request Nov 5, 2024
Open
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

@dqhl76 dqhl76 Awaiting requested review from dqhl76

Assignees
No one assigned
Labels
releases-note/feat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@meteorgan @Xuanwo