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

Issue 225: Implement list streams controller API #242

Merged
merged 16 commits into from
Apr 26, 2021
Merged

Conversation

shrids
Copy link
Contributor

@shrids shrids commented Apr 14, 2021

Change log description

  • Enable List Streams Controller API on the Rust Client.
  • Create a Paginator utility which returns a impl Stream<Item = Result<String, RetryError<ControllerError>>> to handle huge stream lists with pagination.
  • Enable List Streams option in the Controller CLI

Purpose of the change
Fixes #225

What the code does

  • Enable controller api list streams which returns the streams for the provided scope and continuationToken.
async fn list_streams(&self, scope: &Scope, token: &str) -> ResultRetry<Option<(Vec<String>, String)>>;
  • A paginator helper which returns an impl Stream thereby lazily fetch the next page of streams from Pravega
pub fn list_streams(
    scope: Scope,
    client: &dyn ControllerClient,
) -> impl Stream<Item = Result<String, RetryError<ControllerError>>> 

This uses the futures::stream::unfold to fetch streams under a given scope.

  • Example usage
// Sample 1
use pravega_client_shared::Scope;
use pravega_controller_client::paginator::list_streams;
    let stream = list_streams(
        Scope {
            name: "testScope".to_string(),
        },
        controller_client,
    );
    // collect all the Streams in a single vector
    let stream_list:Vec<String> = stream.map(|str| str.unwrap()).collect::<Vec<String>>().await;

// Sample 2
use pravega_client_shared::Scope;
use pravega_controller_client::paginator::list_streams;
    let mut stream = list_streams(
        Scope {
             name: "testScope".to_string(),
         },
        controller_client,
    );
let pravega_stream_1 = stream.next().await;
let pravega_stream_2 = stream.next().await;
// A None is returned at the end of the stream.

CLI Option for list-streams.

osboxes@osboxes:~/$ ./controller-cli list-streams --help
controller-cli-list-streams 0.1.0
List Streams under a scope

USAGE:
    controller-cli list-streams <scope-name>

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

ARGS:
    <scope-name>    Scope Name

How to verify it
The existing and newly added tests should pass.
Verified it for a huge list of streams against standalone.

@codecov-io
Copy link

Codecov Report

Merging #242 (61a0e43) into master (e4f7beb) will increase coverage by 0.24%.
The diff coverage is 80.48%.

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   79.52%   79.77%   +0.24%     
==========================================
  Files          51       51              
  Lines       10102    10208     +106     
==========================================
+ Hits         8034     8143     +109     
+ Misses       2068     2065       -3     
Impacted Files Coverage Δ
controller-client/src/lib.rs 68.41% <80.48%> (+2.11%) ⬆️
src/reactor/segment_writer.rs 85.40% <0.00%> (+0.48%) ⬆️
auth/src/lib.rs 90.42% <0.00%> (+9.78%) ⬆️

@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2021

Codecov Report

Merging #242 (3dc4403) into master (e4f7beb) will decrease coverage by 0.99%.
The diff coverage is 77.23%.

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
- Coverage   79.52%   78.53%   -1.00%     
==========================================
  Files          51       51              
  Lines       10102    10510     +408     
==========================================
+ Hits         8034     8254     +220     
- Misses       2068     2256     +188     
Impacted Files Coverage Δ
controller-client/src/mock_controller.rs 44.98% <0.00%> (-0.45%) ⬇️
src/error.rs 10.81% <0.00%> (-0.31%) ⬇️
src/event_reader.rs 74.70% <0.00%> (ø)
src/reactor/reactors.rs 69.93% <0.00%> (ø)
src/segment_metadata.rs 55.63% <0.00%> (-0.40%) ⬇️
src/tablemap.rs 67.60% <0.00%> (-0.35%) ⬇️
src/raw_client.rs 86.04% <20.00%> (-8.70%) ⬇️
controller-client/src/lib.rs 67.92% <69.87%> (+1.62%) ⬆️
controller-client/src/model_helper.rs 63.63% <90.00%> (+2.63%) ⬆️
src/segment_reader.rs 68.78% <91.66%> (+0.38%) ⬆️
... and 18 more

@shrids shrids marked this pull request as ready for review April 20, 2021 14:22
@Tristan1900
Copy link
Member

Thanks Sandeep! That's really nice!
One thing that is a little bit odd is that list_stream is a controller method but it's not used as controller.list_stream() like other methods. Could we still use controller.list_stream() and let it return the impl stream?

Signed-off-by: Sandeep <[email protected]>
@shrids
Copy link
Contributor Author

shrids commented Apr 21, 2021

Yes @Tristan1900 , that was my initial plan. I later realized that we cannot have impl Stream inside the trait definition.
i.e.

pub trait ControllerClient: Send + Sync {
    fn list_stream(&self, scope: &Scope) -> impl Stream<Item = ResultRetry<String>> // this is not allowed.
}

@Tristan1900
Copy link
Member

@shrids Thanks yeah I didn't know that. Looks like impl trait is not allowed to be in return type. But can we do Box<Stream>?

controller-client/src/lib.rs Outdated Show resolved Hide resolved
controller-client/src/lib.rs Outdated Show resolved Hide resolved
///
/// The below snippets show case the example uses.
/// Sample 1:
///```ignore
Copy link
Member

Choose a reason for hiding this comment

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

Do you want ignore or no_run?

Copy link
Contributor Author

@shrids shrids Apr 22, 2021

Choose a reason for hiding this comment

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

I used ignore since pravega_client module is not visible here

use pravega_client::client_factory::ClientFactory;
  |      ^^^^^^^^^^^^^^ use of undeclared crate or module `pravega_client`

Is there a way around it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, yeah that a different crate. Sorry I don't think there is a way around that which doesn't involve adding a dev-dependency.

controller-client/src/paginator.rs Show resolved Hide resolved
controller-client/src/paginator.rs Outdated Show resolved Hide resolved
controller-client/src/paginator.rs Outdated Show resolved Hide resolved
@tkaitchuck tkaitchuck merged commit a026f15 into master Apr 26, 2021
@tkaitchuck tkaitchuck deleted the issue-list-streams branch April 26, 2021 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement list_streams API
5 participants