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

Keeper Integration API #83

Merged
merged 5 commits into from
Mar 17, 2022
Merged

Keeper Integration API #83

merged 5 commits into from
Mar 17, 2022

Conversation

osopardo1
Copy link
Member

@osopardo1 osopardo1 commented Mar 11, 2022

This PR is changing ONLY the API to connect to the Keeper. It does not introduce any logic.
PR #77 is the one solving #41. Here I only take care of matching calls between the different libraries we want to implement.

In order to delegate the maintenance of the index to a Keeper instance, this is the configuration you need to use:

$SPARK_HOME/bin/spark-shell --master local[*]\
 --packages io.delta:delta-core_2.12:1.0.0,io.qbeast:qbeast-spark-keeper_2.12:0.1.0-a2\
 --jars ./target/scala-2.12/qbeast-spark-assembly-0.2.0.jar\
 --conf spark.sql.extensions=io.qbeast.spark.internal.QbeastSparkSessionExtension\
 --conf spark.qbeast.keeper.host=localhost\
 --conf spark.qbeast.keeper.port=50051

As you can see, there's three new parameters:

1. --packages io.qbeast:qbeast-spark-keeper_2.12:0.1.0-a2 -> Package of the qbeast-spark-keeper driver. It maps calls between the qbeast-spark and qbeast-keeper. It also shades grpc/netty libraries to the Spark.
2. --conf spark.qbeast.keeper.host=localhost -> The ip/host of the keeper
3. --conf spark.qbeast.keeper.port=50051 -> The port of the keeper

@osopardo1 osopardo1 changed the title Changing API to connect to external Keeper Keeper Integration API Mar 11, 2022
@osopardo1 osopardo1 requested a review from eavilaes March 11, 2022 14:21
@osopardo1 osopardo1 marked this pull request as ready for review March 11, 2022 14:29
Copy link
Contributor

@eavilaes eavilaes left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Should we add some tests to check that --conf spark.qbeast.keeper.XXX is used in the Keeper?

@osopardo1 osopardo1 requested a review from cugni March 15, 2022 09:02
Copy link
Member

@cugni cugni left a comment

Choose a reason for hiding this comment

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

I have some doubts about how the keeper service is defined. Also, I can't find documentation explaining how (at least not in this PR 😅)

@Qbeast-io Qbeast-io deleted a comment from osopardo1 Mar 16, 2022
Copy link
Member

@cugni cugni left a comment

Choose a reason for hiding this comment

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

My bad, I misread the cood. It seems all good 👍🏻

@osopardo1 osopardo1 merged commit 20b018b into Qbeast-io:main Mar 17, 2022
@osopardo1 osopardo1 deleted the keeper-api branch March 25, 2022 12:13
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.

3 participants