search

SemyonSinchenko / spark-connect-example

An example of SparkConnect extension.
11 stars 2 forks source link

Support multiple commands in demo #1

Open bcb44-esri opened 1 month ago

bcb44-esri commented 1 month ago

Description:

I'm generally a little confused about how to have multiple plugins at the same time running. I decided to start w multiple commands since that's the simplest and it seems like there's an error in the sample code (though full disclosure, I might be missing something)

In the spark codebase, you can register multiple commands and then the spark connect server will iterate over all of them, calling process() on them until one returns true (spark source)

Because of this, it seems like commands are supposed to return false if the message type doesn't match the expected type, indicating that there's another command that the message is for

In the example here, it returns true on an unmatched input type which would override other plugins later in the chain that the message is actually meant for https://github.com/SemyonSinchenko/spark-connect-example/blob/e2e60e5cb1bfbc12571309bf167f01d4347a6063/src/main/java/com/ssinchenko/example/server/CommandLikeLogicPlugin.java#L21-L32

Based on this, it seems like it should return false at the end but again, this isn't super well doc-ed so kinda guessing here. The other extension types wrap in an optional so its not a problem there. Does all of that sound right?

bcb44-esri commented 1 month ago

I'm only implementing one plugin so I can do a more complicated if else w a bunch of message type matching to handle all the commands in one class. But we're using other spark plugins that are going to need to use spark connect and I want to make sure I design this so it'll be compatible w them

SemyonSinchenko commented 1 month ago

Based on this, it seems like it should return false at the end but again, this isn't super well doc-ed so kinda guessing here. The other extension types wrap in an optional so its not a problem there. Does all of that sound right?

Yes, it sounds right from the first look. I will update an example soon, thanks for the report

SemyonSinchenko commented 1 month ago

@bcb44-esri I would recommend you also to check my another blog-post, especially the latest part, related to problem with shading rules in Spark 3.5.x; also there is a bug with pom.xml in 3.5.1 and 3.4.x, so 3.5.2 is the most working version if you want to do connect plugin.

P.S. The code from this repository will be moved into another place soon. I and @MrPowers are working on creating a meta-repository with examples of how-to with Connect. I will address your points about returned values during the migration.

bcb44-esri commented 1 month ago

Ah that's awesome, thanks for leading the charge on this. Connect seems pretty well designed but the lack of docs on actually writing stuff for it is a huge pain. Without your guides, I would've given up a while ago lol