Skip to content
New issue

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

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

Already on GitHub? # to your account

core: Add start-after for OpList and add into capability #2069

Closed
Xuanwo opened this issue Apr 22, 2023 · 7 comments · Fixed by #2071
Closed

core: Add start-after for OpList and add into capability #2069

Xuanwo opened this issue Apr 22, 2023 · 7 comments · Fixed by #2071
Assignees

Comments

@Xuanwo
Copy link
Member

Xuanwo commented Apr 22, 2023

No description provided.

@Xuanwo Xuanwo changed the title core: Add start-after for OpList and add into capability @lexcao core: Add start-after for OpList and add into capability Apr 22, 2023
@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 22, 2023

cc @lexcao

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 22, 2023

We can discuss the detials in this issue

@lexcao
Copy link
Contributor

lexcao commented Apr 22, 2023

Hi @Xuanwo , this is my first time contributing to OpenDAL, I would like to discuss the details before coding, please feel free to correct me.

To support start-after param for List operation, we could make these changes:

  1. Add start_after arg (with getter and setter) for OpList
  2. Add list_with method with param OpList for Operator

I need help on how to

add into capability

What does that mean?

I just found the Capability is used on operations rather than a single argument.

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 22, 2023

add into capability

We need to add a new field called list_with_start_after: bool to indicate that service support this arg.

@lexcao
Copy link
Contributor

lexcao commented Apr 22, 2023

Thanks.
I find there is a option for batch parameter max_batch_operations in AccessorInfo

So the list_with_start_after is added to the AccessorInfo?
Then to add a warning log when the accessor does not support the arg but is presented in the OpList when calling list_with method.


I just found there is a way to support custom args by leveraging AccessorCapability

enum AccessorCapability {
    List { start_after: bool }
    ...
}

This needs to extend AccessorCapability to support this since currently it is just u32 enum.

With that, the max_batch_operations could be replaced by

Batch { max_operations: uszie }

But this is another topic, I don't know if this is a good practice or not.

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 22, 2023

I'm guessing you need to update the main branch. We have a new Capacity design now.

@lexcao
Copy link
Contributor

lexcao commented Apr 22, 2023

Great, thank you! My bad.
I am ready to code. 💪

lexcao added a commit to lexcao/incubator-opendal that referenced this issue Apr 22, 2023
Closes apache#2069
This PR makes following changes:
1. added `start_after` argument for `OpList`.
2. added `list_with` method for `Operator`.
3. added `list_with_start_after` option for `Capability`.
Xuanwo pushed a commit that referenced this issue Apr 22, 2023
Closes #2069
This PR makes following changes:
1. added `start_after` argument for `OpList`.
2. added `list_with` method for `Operator`.
3. added `list_with_start_after` option for `Capability`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants