Hi folks, I saw there was a recent change aimed at detecting backwards incompat api changes for pinot-spi - I found this makes it pretty difficult to add more args for index config constructors. Eg., trying to add a new arg for JsonIndexConfig, we now see the error:

Error:  Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.23.1:cmp (default) on project pinot-spi: There is at least one incompatibility: org.apache.pinot.spi.config.table.JsonIndexConfig.JsonIndexConfig(java.lang.Boolean,int,boolean,boolean,java.util.Set,java.util.Set,java.util.Set,java.util.Set,int,boolean):CONSTRUCTOR_REMOVED -> [Help 1]

Any concerns with excluding some config classes (or parent config package) from this check? or should we be creating a new constructor for every config addition going forward?

@Chris Peck - Which PR of yours is getting caught by SPI incompatible change verifier ?

This is the change: https://github.com/apache/pinot/pull/15685/files#diff-2a120570b1313f73e43e017eb6e91c9f833c8301e0b1fb818d7d57725f18211fL83

I see. Let me respond here in a bit

+1 to this, https://github.com/apache/pinot/pull/15606/files#diff-342025c1783e6c36d8b61a3e2c986bedefb13cd5cd559018a1dd89b992f41061R73 E.g. I cannot add a new argument into

QueryConfig

and I have to create a new constructor and mark it as

@JsonCreator

and remove this annotation from the existing constructor

So the purpose of that work was not to block changes. We really need to be conscious about making breaking changes to the SPI otherwise consumers keep fixing their builds repeatedly. We have had a very hard time in coping up with such changes together with all the dependency stuff (we are implementing a verifier for that too to enforce the guidelines that community had aligned on). Do you have any suggestions on what things should be relaxed from this verifier ? I feel we have been defeating the purpose of SPI for quite some time now by changing it in arbitrary ways - method name changes, return type changes etc. It's not the standard practice of a stable API and this way no one will be able to work off master.

What do you think about excluding based on

@JsonCreator

? It seems these constructors are generally used to parse JSON rather than directly called and this would keep changes like the one I'm making and Xiang's straightforward

I took a look at PR. It' sunfortunately resetting the progress by excluding the entire config package 😞