Hi Chris, Thanks for taking another look!
That's a good point which I should have mentioned in the KIP. I think we can have a PossiblyVersioned interface that extends Versioned and has the default implementation. I'd like to also print a warning at startup if a plugin does not override version(). Thanks for the suggestion! I'm even considering whether PossiblyVersioned could be deprecated immediately in order to be removed in the next major release. If so, the warning message I mentioned would state that plugins not overriding version() would stop working in the next major release. Thanks, Mickael On Mon, Jan 24, 2022 at 5:41 PM Mickael Maison <mickael.mai...@gmail.com> wrote: > > Hi Tom, > > Thanks for the feedback. > > You're right we don't have any contracts around the version field. > Like Chris I've only used the current field to find the exact source > code of connectors. Even if it's a limited use, I think that's enough > to justify adding it to all plugins and making them consistent. Work > for improving the contract can be done in a separate KIP. > > Thanks, > Mickael > > On Wed, Jan 19, 2022 at 7:55 PM Chris Egerton > <chr...@confluent.io.invalid> wrote: > > > > Hi all, > > > > With regards to Tom's question about possible uses of the version string, > > one of the most useful applications I've found for it so far has been > > debugging connector issues, especially based on captured logs. Using the > > version string to identify a git tag containing the exact source code for a > > connector, and then using that as a reference when poring over log files > > that usually contain class names, method names, and/or line numbers, is a > > great way to construct a timeline of events and understand how things may > > have gone wrong. I don't think we can require this as part of the contract > > for the version method, but at least identifying this use case in the docs > > for it could be useful to help connector/SMT/converter/etc. developers > > understand why they should care about what this method returns and how it > > could be beneficial to themselves and their users down the road. > > > > One other potential use case that comes up is that, if/when Connect > > supports loading multiple versions of the same plugin type on a single > > worker, the version string can be used to identify plugins that share the > > same name and type but which should be exposed individually to users. If we > > want to leave room for this possibility, perhaps part of the contract for > > the method can be that "any two releases of the same plugin but with > > different behavior should return different values from their version > > method". > > > > One final note--I've just realized that, with the introduction of a default > > implementation for the Versioned interface, we've made Connector::version > > and ConnectRestExtension::version optional to implement, whereas before it > > was required that instances of these plugins explicitly implement that > > method. Should we allow this? An alternative could be to add a new > > "PossiblyVersioned" interface (name obviously subject to change) that > > extends the current Versioned interface and contains a default > > implementation that returns the "undefined" string. The to-be-updated > > plugin interfaces could extend from this new "PossiblyVersioned" interface, > > and the plugin interfaces that extend from the current "Versioned" > > interface could be left unchanged. > > > > Cheers, > > > > Chris > > > > On Wed, Jan 19, 2022 at 10:04 AM Tom Bentley <tbent...@redhat.com> wrote: > > > > > Hi Mickael, > > > > > > Thanks for the KIP. Overall I think this looks good. > > > > > > I have a small concern about the proliferation of Versioned. Currently > > > this > > > interface has a really weak contract. There's no requirement for versions > > > to be comparable, or documentation about when versions should be changed > > > by > > > authors of plugins which extend this interface. For example, if I release > > > version 1.2.3 of my SMT should the version be 1.2.3 or something else? Is > > > compatibility implied for different versions of my jar file which return > > > the same result from version()? Because of this it's not really clear what > > > the consumer of the version string is supposed to do with it. So I think > > > if > > > we're going to be using Versioned more extensively it would be good to > > > make > > > the expectations clearer, for both authors and people wanting to reason > > > about versions. > > > > > > Many thanks, > > > > > > Tom > > > > > > On Mon, 6 Dec 2021 at 09:56, Mickael Maison <mickael.mai...@gmail.com> > > > wrote: > > > > > > > Hi Arjun, > > > > > > > > Thank you for taking a look at this KIP. > > > > > > > > 1.a Yes this is exactly what this KIP is proposing. > > > > > > > > 1.b I prefer returning all plugins together instead of having different > > > > paths. > > > > > > > > 2. While we seem to agree it would be nice to expose worker plugins, I > > > > don't think we've reached a consensus on the right way to do it. For > > > > that reason, I'm dropping this feature from this KIP. We can address > > > > this issue in another KIP. I have updated the KIP. > > > > > > > > Thanks, > > > > Mickael > > > > > > > > On Mon, Dec 6, 2021 at 10:15 AM Mickael Maison <mickael.mai...@gmail.com > > > > > > > > wrote: > > > > > > > > > > Thanks Randall, updated. > > > > > > > > > > On Sun, Dec 5, 2021 at 6:05 AM Arjun Satish <arjun.sat...@gmail.com> > > > > wrote: > > > > > > > > > > > > hey folks, > > > > > > > > > > > > Thanks a lot for the KIP and the discussions. Here are a couple of > > > > thoughts > > > > > > (apologies if I missed some point in this thread). > > > > > > > > > > > > 1. it seems like we are changing the behavior of /connector-plugins > > > by > > > > now > > > > > > returning the list of transforms, converters as well? This would > > > break > > > > > > backward compatibility, and clients would need to track what version > > > of > > > > > > Connect workers they are querying to keep their experience > > > consistent. > > > > I > > > > > > wonder if we can keep the old behavior, with one of the following > > > > methods: > > > > > > > > > > > > 1a. We add an "expand" multi-query to the /connector-plugin endpoint > > > > like > > > > > > how it was done in KIP-465 [1]. This would preserve backward > > > > compatibility, > > > > > > as well as allow you to get consolidated output. Acceptable values > > > for > > > > the > > > > > > new param could be: "all" (to show all plugins), or > > > > > > "connectors|transforms|predicates" that show a subset of the plugin > > > > types. > > > > > > > > > > > > 1b. Query params might get crummy to extend. Alternatively, we could > > > > have > > > > > > an additional fine-grained paths for each plugin: > > > > > > > > > > > > # to list all the source and sink plugins > > > > > > /connector-plugins/connectors > > > > > > > > > > > > # to list all the SMT plugins > > > > > > /connector-plugins/transformations > > > > > > > > > > > > # to list all the predicate plugins > > > > > > /connector-plugins/predicates > > > > > > > > > > > > # to list all the converter plugins > > > > > > /connector-plugins/converters > > > > > > > > > > > > # to list all the header converter plugins > > > > > > /connector-plugins/header-converters > > > > > > > > > > > > This allows us to cleanly add new types of plugins down the road, > > > > > > and > > > > > > naturally extend to displaying attributes of these objects. Configs, > > > > for > > > > > > example, can be accessed with > > > > > > > > > > > > > > > > > > > http://localhost:8083/connector-plugins/transformations/Cast$Value/config > > > > > > > > > > > > A migration path (for both the suggestions above) would be to add a > > > > > > deprecation notice on the current behavior of /connector-plugins > > > > (where, > > > > > > after this PR is merged, both /connector-plugins and > > > > > > /connector-plugins/connectors return the same output). And starting > > > > from > > > > > > the next major version release, /connector-plugins redirects to > > > > > > /connector-plugins/connectors (making it super easy for clients to > > > > catch > > > > > > up). > > > > > > > > > > > > 2. Regarding /worker-plugins, I agree that it is useful to have the > > > > REST > > > > > > API inform the state of the REST extensions, and Config Providers. > > > > However, > > > > > > users of the Connect clusters will rarely need to know how a config > > > > > > provider or REST extension is configured to construct data > > > > > > pipelines. > > > > It's > > > > > > mostly an admin task, AFAICT. Also, we have also seen these problems > > > in > > > > > > Connect clusters where one host is somewhat differently set up than > > > > other > > > > > > (contents of plugin path, classpath, local worker configuration > > > files, > > > > jvm > > > > > > versions, GC settings) that surface as issues in Connector behavior. > > > > > > Overall, I'm wondering if we should just tackle this problem in a > > > > separate > > > > > > KIP, that evaluates all of these cases. All of this might nicely > > > > > > fall > > > > into > > > > > > under the /admin endpoint and allow us to expose detailed info on > > > > > > the > > > > state > > > > > > of the worker (maybe a /admin/worker/ that returns all of this > > > > > > info). > > > > > > > > > > > > Thanks, > > > > > > > > > > > > [1] > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-465%3A+Add+Consolidated+Connector+Endpoint+to+Connect+REST+API > > > > > > > > > > > > > > > > > > On Sat, Dec 4, 2021 at 10:29 AM Randall Hauch <rha...@gmail.com> > > > > wrote: > > > > > > > > > > > > > Thanks, Mickael. Can we call out these as literals? For example: > > > > > > > > > > > > > > > The response structure of the objects in the array remain > > > > unchanged, but > > > > > > > this proposal does add "converter", "header_converter" and > > > > "transformation" > > > > > > > to the allowed values for the "type" field, in addition to the > > > > existing > > > > > > > "sink" and "source" values. > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > Randall > > > > > > > > > > > > > > On Sat, Dec 4, 2021 at 4:58 AM Mickael Maison < > > > > mickael.mai...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Randall, > > > > > > > > > > > > > > > > Sure. The types are sink, source, converter, header_converter, > > > > > > > > transformation and predicate. I've updated the KIP. > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Mickael > > > > > > > > > > > > > > > > On Fri, Dec 3, 2021 at 10:14 PM Randall Hauch <rha...@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > > > > One more thing, inline below: > > > > > > > > > > > > > > > > > > Randall > > > > > > > > > > > > > > > > > > On Fri, Dec 3, 2021 at 10:38 AM Mickael Maison < > > > > > > > mickael.mai...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > Thanks Randall for the feedback! > > > > > > > > > > > > > > > > > > > > 1. Done > > > > > > > > > > > > > > > > > > > > 2. Yes that makes sense. I've updated the KIP > > > > > > > > > > > > > > > > > > > > 3.a I've updated to explicitly mention the plugin can be > > > > provided > > > > > > > > > > using the fully qualified class or the alias. > > > > > > > > > > 3.b Only concrete classes will be returned. I made the > > > mistake > > > > when I > > > > > > > > > > crafted the examples by hand. Fixed > > > > > > > > > > > > > > > > > > Can we also enumerate all of the values that can be included > > > > > > > > > in > > > > the > > > > > > > > > "type" field in the response from the "GET > > > > > > > > > /connector-plugins?connectorsOnly=false"? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. If a plugin does not override config(), the response is > > > > > > > > > > an > > > > empty > > > > > > > > > > array. I've updated the KIP > > > > > > > > > > > > > > > > > > > > 5. I have updated the compatibility section with more > > > > > > > > > > details > > > > > > > > > > > > > > > > > > > > 6. Can you clarify what you mean? KIP-507 was for securing > > > > internal > > > > > > > > > > REST endpoints. > > > > > > > > > > > > > > > > > > > > 7. I don't think it is necessary. I think the use case Chris > > > > > > > mentioned > > > > > > > > > > could be useful for admins. I'm interested in exposing the > > > > > > > > > > connector-level plugins and their configs. I thought it was > > > > easy to > > > > > > > > > > add an API for worker plugins while working in this area but > > > > if there > > > > > > > > > > isn't a strong consensus, I'm happy to drop it. > > > > > > > > > > > > > > > > > > > > Mickael > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 1, 2021 at 5:39 PM Randall Hauch < > > > rha...@gmail.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP, Mickael. And thanks to everyone else > > > for > > > > the > > > > > > > good > > > > > > > > > > > discussion so far. > > > > > > > > > > > > > > > > > > > > > > Overall this is pretty good, but I do have a few > > > > > > > questions/comments: > > > > > > > > > > > 1. Can we include the HTTP method/verb in each of the > > > > endpoints in > > > > > > > the > > > > > > > > > > > "Public Interfaces" section? > > > > > > > > > > > 2. The sample output for `GET > > > > > > > /connector-plugins?connectorsOnly=false` > > > > > > > > > > > includes a `version` field for the connector plugin, but > > > not > > > > for > > > > > > > the > > > > > > > > > > > transform or converter plugins. Neither the > > > `Transformation` > > > > or > > > > > > > > > > > `Converter` interfaces have a version nor implement the > > > > `Versioned` > > > > > > > > > > > interface, so should we add the `Versioned` interface to > > > the > > > > > > > > > > > `Transformation` and `Converter` interfaces and add a > > > default > > > > > > > method > > > > > > > > > > > that returns "unknown"? Yes, we could do that in a > > > > > > > > > > > separate > > > > KIP, > > > > > > > but I > > > > > > > > > > > think it's appropriate to do it in this KIP for a few > > > > reasons. > > > > > > > First, > > > > > > > > > > > we're already modifying the `Converter` interface in this > > > > KIP and > > > > > > > it > > > > > > > > > > > would be quite straightforward to also extend the > > > `Versioned` > > > > > > > > > > > interface and add a default method; then doing that for > > > > > > > > > > > `Transformation` would also be relatively straightforward. > > > > Second, > > > > > > > > > > > this KIP provides the justification for adding a version > > > > > > > > > > > to > > > > the > > > > > > > > > > > `Transformation` and `Converter` interfaces. > > > > > > > > > > > 3. In the "Public Interfaces" section, there are two > > > > > > > inconsistencies > > > > > > > > > > > between the `GET /connector-plugins` and `GET > > > > > > > > > > > /connector-plugins/<plugin>/config` methods. Both will > > > > > > > > > > > make > > > > it > > > > > > > > > > > difficult to use the second method, since the input to the > > > > second > > > > > > > > > > > method is not directly found in the response from the > > > > > > > > > > > first > > > > method: > > > > > > > > > > > a. The example for `GET > > > > /connector-plugins/Cast$Value/config` uses > > > > > > > > > > > short name, or what Connect often refers to as the alias, > > > > > > > "Cast$Value" > > > > > > > > > > > for the plugin, rather than the fully-qualified name > > > > > > > > > > > "org.apache.kafka.connect.transforms.Cast$Value". The KIP > > > > should > > > > > > > > > > > specify whether it will support both the shortened name > > > > > > > > > > > and > > > > the > > > > > > > > > > > fully-qualified name, or whether only one of these will be > > > > > > > supported. > > > > > > > > > > > Not supporting the fully-qualified name will make it > > > > impossible to > > > > > > > use > > > > > > > > > > > the response from the first method as an input to the > > > second. > > > > > > > > > > > a. The example for the list method returns the > > > > `Transformation` > > > > > > > base > > > > > > > > > > > and abstract classes (e.g., > > > > > > > > > > > "org.apache.kafka.connect.transforms.Cast") rather than > > > > > > > > > > > the > > > > > > > concrete > > > > > > > > > > > classes (e.g., > > > > "org.apache.kafka.connect.transforms.Cast$Value" and > > > > > > > > > > > "org.apache.kafka.connect.transforms.Cast$Key"), while the > > > > `GET > > > > > > > > > > > /connector-plugins/<plugin>/config` will require the > > > concrete > > > > > > > class. > > > > > > > > > > > 4. The "Public Interfaces" section's description of the > > > `GET > > > > > > > > > > > /connector-plugins/<plugin>/config` method should probably > > > > specify > > > > > > > the > > > > > > > > > > > expected behavior for `Converter` implementations that > > > don't > > > > > > > override > > > > > > > > > > > the new `config()` method. > > > > > > > > > > > 5. The "Compatibility, Deprecation, and Migration Plan" > > > > section > > > > > > > should > > > > > > > > > > > describe whether older `Converter` implementations > > > (compiled > > > > with > > > > > > > > > > > older versions of the Connect API without Converter > > > extending > > > > > > > > > > > Configurable) will be able to be installed into newer > > > Connect > > > > > > > > > > > runtimes, and what the behavior will be for the `GET > > > > > > > > > > > /connector-plugins/<plugin>/config` method. (Although my > > > > suggestion > > > > > > > > > > > for #5 will partially address the latter, we still need to > > > > talk > > > > > > > about > > > > > > > > > > > it in terms of compatibility.) > > > > > > > > > > > 6. Can we add a small section addressing the security > > > > implications > > > > > > > of > > > > > > > > > > > these new APIs? KIP-507 [1] applied to APIs that existed > > > > > > > > > > > at > > > > the > > > > > > > time, > > > > > > > > > > > and this KIP should at least mention how it aligns with > > > that > > > > > > > > > > > mechanism. > > > > > > > > > > > 7. Do we really think adding `GET /worker-plugins` is > > > > necessary? > > > > > > > While > > > > > > > > > > > I understand the reasoning, I'm less convinced that it's > > > > worth > > > > > > > adding > > > > > > > > > > > to the public API. First, an administrator still has to > > > > modify the > > > > > > > > > > > worker config for all workers to take advantage of that. > > > > Second, > > > > > > > > > > > adding this method increases the surface area and > > > > > > > > > > > therefore > > > > risk of > > > > > > > > > > > additional attack vectors, despite not really being able > > > > > > > > > > > to > > > > use the > > > > > > > > > > > resulting information in any other part of the API. > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > > > > > > > > Randall > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-507%3A+Securing+Internal+Connect+REST+Endpoints > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 1, 2021 at 7:52 AM Mickael Maison < > > > > > > > mickael.mai...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > > > > > > > Thanks again for the feedback! I've updated the KIP > > > > > > > > > > > > based > > > > on our > > > > > > > last > > > > > > > > > > > > discussions. I've decided to include the new endpoint > > > > > > > > > > > > for > > > > worker > > > > > > > > > > > > plugins. > > > > > > > > > > > > > > > > > > > > > > > > 1. Yes I agree, it's best to gate the new behavior. > > > > > > > > > > > > 2. Yes, it was a remnant from the original proposal. > > > > > > > > > > > > I've > > > > now > > > > > > > removed > > > > > > > > > > > > the location field. > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Mickael > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 30, 2021 at 3:22 AM Chris Egerton > > > > > > > > > > > > <chr...@confluent.io.invalid> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Mickael, > > > > > > > > > > > > > > > > > > > > > > > > > > I think that's a great idea! I especially like how we > > > can > > > > > > > establish the > > > > > > > > > > > > > expectation that any plugin type that appears in the > > > > response > > > > > > > from the GET > > > > > > > > > > > > > /connector-plugins endpoint will have a corresponding > > > GET > > > > > > > > > > > > > /connector-plugins/<type>/config endpoint, but (if we > > > > decide > > > > > > > to add them in > > > > > > > > > > > > > the future), worker plugins won't be expected to > > > > > > > > > > > > > expose > > > > this > > > > > > > kind of > > > > > > > > > > > > > information and the different root path helps give a > > > > decent > > > > > > > hint about this. > > > > > > > > > > > > > > > > > > > > > > > > > > I also like the choice to return an empty ConfigDef > > > from > > > > > > > Converter::config > > > > > > > > > > > > > instead of null. > > > > > > > > > > > > > > > > > > > > > > > > > > Two things come to mind: > > > > > > > > > > > > > > > > > > > > > > > > > > 1. We may want to gate this behind a URL query > > > parameter > > > > > > > (maybe something > > > > > > > > > > > > > like "connectorsOnly") that defaults to the old > > > behavior > > > > in > > > > > > > order to avoid > > > > > > > > > > > > > breaking existing tools such as programmatic UIs that > > > > use the > > > > > > > endpoint > > > > > > > > > > > > > today to discover the connectors that can be created > > > > > > > > > > > > > by > > > > the > > > > > > > user. We can > > > > > > > > > > > > > even plan to change the default for that parameter to > > > the > > > > > > > newly-proposed > > > > > > > > > > > > > behavior in the next major release, which should give > > > > people > > > > > > > enough time to > > > > > > > > > > > > > either adapt to the expanded response format or add > > > > > > > > > > > > > the > > > > query > > > > > > > parameter to > > > > > > > > > > > > > their tooling. > > > > > > > > > > > > > > > > > > > > > > > > > > 2. The existing GET /connector-plugins endpoint > > > > > > > > > > > > > doesn't > > > > > > > contain information > > > > > > > > > > > > > on the location of the plugin on the worker's file > > > > system. Do > > > > > > > you think we > > > > > > > > > > > > > should still include this info in the new response > > > > format? > > > > > > > Correct me if > > > > > > > > > > > > > I'm wrong but it seems it may have been proposed > > > > originally to > > > > > > > help prevent > > > > > > > > > > > > > already-addressed bugs in Connect classloading from > > > > striking; > > > > > > > all else > > > > > > > > > > > > > equal, I'd personally err on the side of leaving this > > > > info out > > > > > > > or at least > > > > > > > > > > > > > reducing permitted values for it to just "classpath" > > > > > > > > > > > > > or > > > > > > > "plugin path" in > > > > > > > > > > > > > order to avoid leaking worker file paths into the REST > > > > API, > > > > > > > which might > > > > > > > > > > > > > bother super security-conscious users. > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 29, 2021 at 5:52 AM Mickael Maison < > > > > > > > mickael.mai...@gmail.com> > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes to keep compatibility we want a default > > > > implementation > > > > > > > for > > > > > > > > > > > > > > Converter.configs(), I've updated the KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regarding worker plugins, the use case you described > > > > seems > > > > > > > valuable. > > > > > > > > > > > > > > I'd prefer not mixing worker and connector plugins > > > > > > > > > > > > > > on > > > > the > > > > > > > same > > > > > > > > > > > > > > endpoint but I agree using /plugins and > > > /worker-plugins > > > > > > > could be > > > > > > > > > > > > > > confusing. > > > > > > > > > > > > > > > > > > > > > > > > > > > > One alternative is to expose all connector-level > > > > plugins via > > > > > > > the > > > > > > > > > > > > > > existing /connector-plugins endpoint. In that case, > > > > we'd > > > > > > > need to keep > > > > > > > > > > > > > > the current JSON schema and not group plugins by > > > type. > > > > As > > > > > > > the current > > > > > > > > > > > > > > schema already has a type field for each entry, > > > > > > > > > > > > > > we'll > > > > still > > > > > > > be able to > > > > > > > > > > > > > > tell them apart. Then we can have /worker-plugins > > > and a > > > > > > > relatively > > > > > > > > > > > > > > clean API. What do you think? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Mickael > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Nov 28, 2021 at 8:21 PM Chris Egerton > > > > > > > > > > > > > > <chr...@confluent.io.invalid> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Mickael, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think one potential use case for exposing > > > > worker-level > > > > > > > plugins is that > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > may make it easier to determine whether a worker > > > > > > > > > > > > > > > is > > > > set up > > > > > > > correctly (the > > > > > > > > > > > > > > > current alternative requires looking through log > > > > files and > > > > > > > can be a > > > > > > > > > > > > > > little > > > > > > > > > > > > > > > tedious), and might even make it possible to > > > > automatically > > > > > > > identify > > > > > > > > > > > > > > > discrepancies within a cluster by diffing the > > > > contents of > > > > > > > that endpoint > > > > > > > > > > > > > > > across each worker. But I don't think this has to > > > be > > > > > > > addressed by the > > > > > > > > > > > > > > > current KIP; the only thing that bothers me a > > > little > > > > is > > > > > > > that "plugins" is > > > > > > > > > > > > > > > generic and it may confuse people down the road if > > > > we add > > > > > > > an endpoint for > > > > > > > > > > > > > > > worker-level plugins ("why is one just called > > > > 'plugins' > > > > > > > and the other one > > > > > > > > > > > > > > > is 'worker-plugins'?"). Probably not worth > > > > > > > > > > > > > > > blocking > > > > on, > > > > > > > though. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Agreed that the suggestion for improved validation > > > > should > > > > > > > be made on the > > > > > > > > > > > > > > > KIP-802 thread. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I also noticed that the newly-proposed config > > > method > > > > for > > > > > > > the Converter > > > > > > > > > > > > > > > interface doesn't have a default implementation, > > > > making it > > > > > > > > > > > > > > > backwards-incompatible. Should we add a default > > > > > > > implementation that > > > > > > > > > > > > > > returns > > > > > > > > > > > > > > > either null or an empty ConfigDef? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Nov 26, 2021 at 8:35 AM Mickael Maison < > > > > > > > mickael.mai...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. If we want to expose worker plugins, I think > > > we > > > > > > > should do it via a > > > > > > > > > > > > > > > > separate endpoint. But to be honest, I'm not > > > > > > > > > > > > > > > > even > > > > sure I > > > > > > > see strong > > > > > > > > > > > > > > > > use cases for exposing them as they are either > > > > enabled > > > > > > > or not and > > > > > > > > > > > > > > > > can't be changed at runtime. So I'd prefer to > > > > stick to > > > > > > > "connector > > > > > > > > > > > > > > > > level" plugins in this KIP. Let me now if you > > > have > > > > use > > > > > > > cases, I'm open > > > > > > > > > > > > > > > > to reconsider this choice. > > > > > > > > > > > > > > > > I'll add that in the rejected alternatives > > > section > > > > for > > > > > > > now > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. I remembered seeing issues in the past with > > > > multiple > > > > > > > plugin.path > > > > > > > > > > > > > > > > entries but I tried today and I was able to mix > > > and > > > > > > > match plugins from > > > > > > > > > > > > > > > > different paths. So my bad for getting confused. > > > > > > > > > > > > > > > > Then I agree, it makes more sense to group them > > > by > > > > > > > plugin type. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. Yes this should be covered in KIP-802: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-802%3A+Validation+Support+for+Kafka+Connect+SMT+Options > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. No particular reason. We can support both > > > > formats > > > > > > > like today. I've > > > > > > > > > > > > > > > > updated the KIP > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Mickael > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 23, 2021 at 6:40 PM Chris Egerton > > > > > > > > > > > > > > > > <chr...@confluent.io.invalid> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Mickael, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think the increase in scope here is great > > > > > > > > > > > > > > > > > and > > > > the > > > > > > > added value > > > > > > > > > > > > > > certainly > > > > > > > > > > > > > > > > > justifies the proposed changes. I have some > > > > thoughts > > > > > > > but overall I > > > > > > > > > > > > > > like > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > direction this is going in now. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. The new /plugins endpoint is described as > > > > > > > containing "all plugins > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > are Connectors, Transformations, Converters, > > > > > > > HeaderConverters and > > > > > > > > > > > > > > > > > Predicates". So essentially, it looks like we > > > > want to > > > > > > > expose all > > > > > > > > > > > > > > plugins > > > > > > > > > > > > > > > > > that are configured on a per-connector basis, > > > but > > > > > > > exclude plugins > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > are > > > > > > > > > > > > > > > > > configured on a per-worker basis (such as > > > config > > > > > > > providers and REST > > > > > > > > > > > > > > > > > extensions). Do you think it may be valuable > > > > > > > > > > > > > > > > > to > > > > expose > > > > > > > information on > > > > > > > > > > > > > > > > > worker-level plugins as well? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. The description for the new /plugins > > > endpoint > > > > also > > > > > > > states that > > > > > > > > > > > > > > > > "Plugins > > > > > > > > > > > > > > > > > will be grouped by plugin.path. This will make > > > it > > > > > > > clear to users > > > > > > > > > > > > > > what's > > > > > > > > > > > > > > > > > available to use as it's not possible to use a > > > > > > > Connector from one > > > > > > > > > > > > > > path > > > > > > > > > > > > > > > > with > > > > > > > > > > > > > > > > > Transformations from another.". Is this true? > > > > > > > > > > > > > > > > > I > > > > > > > thought that > > > > > > > > > > > > > > Connect's > > > > > > > > > > > > > > > > > classloading made it possible to package > > > > > > > > > > > > > > > > > converters/transformations/predicates > > > completely > > > > > > > independently from > > > > > > > > > > > > > > each > > > > > > > > > > > > > > > > > other, and to reference them from > > > > > > > also-independently-packaged > > > > > > > > > > > > > > connectors. > > > > > > > > > > > > > > > > > If it turns out that this is the case, could > > > > > > > > > > > > > > > > > we > > > > > > > consider > > > > > > > > > > > > > > restructuring > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > response to be grouped by plugin type instead > > > of > > > > by > > > > > > > classloader? > > > > > > > > > > > > > > There's > > > > > > > > > > > > > > > > > also the ungrouped format proposed in KIP-494 > > > > > > > > > > > > > > > > > ( > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=120740150 > > > > > > > > > > > > > > > > ) > > > > > > > > > > > > > > > > > which we might consider as well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. I think this can be left for a follow-up > > > > > > > > > > > > > > > > > KIP > > > > if > > > > > > > necessary, but I'm > > > > > > > > > > > > > > > > > curious about your thoughts on adding new > > > > validate > > > > > > > methods to all > > > > > > > > > > > > > > > > > connector-level plugins that can be used > > > > similarly to > > > > > > > how the > > > > > > > > > > > > > > existing > > > > > > > > > > > > > > > > > Connector::validate method ( > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/1e0916580f16b99b911b0ed36e9740dcaeef520e/connect/api/src/main/java/org/apache/kafka/connect/connector/Connector.java#L131-L146 > > > > > > > > > > > > > > > > ) > > > > > > > > > > > > > > > > > is used. This would allow for plugins to > > > perform > > > > > > > validation that's > > > > > > > > > > > > > > more > > > > > > > > > > > > > > > > > sophisticated than what the ConfigDef is > > > capable > > > > of, > > > > > > > such as > > > > > > > > > > > > > > validating > > > > > > > > > > > > > > > > > combinations of properties like a hostname and > > > > > > > credentials for > > > > > > > > > > > > > > reaching > > > > > > > > > > > > > > > > it. > > > > > > > > > > > > > > > > > I know that at least Confluent's Avro, > > > protobuf, > > > > and > > > > > > > JSON schema > > > > > > > > > > > > > > > > converters > > > > > > > > > > > > > > > > > would benefit from this kind of feature. It's > > > > > > > > > > > > > > > > > a > > > > little > > > > > > > tangential to > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > KIP (which at the moment is about discovering > > > > plugins > > > > > > > and their > > > > > > > > > > > > > > > > > configuration surfaces, as opposed to > > > validating > > > > > > > them), but I > > > > > > > > > > > > > > figured I'd > > > > > > > > > > > > > > > > > ask since we're going to be expanding the > > > > Converter > > > > > > > interface and it > > > > > > > > > > > > > > may > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > useful to tackle this while we're in the > > > > neighborhood. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. The description for the new > > > > > > > /plugins/<type>/<name>/configdef > > > > > > > > > > > > > > endpoint > > > > > > > > > > > > > > > > > states that "Name must be the fully qualified > > > > class > > > > > > > name of the > > > > > > > > > > > > > > plugin". > > > > > > > > > > > > > > > > > Any reason not to also support aliases (e.g., > > > > > > > > > > > > > > "FileStreamSinkConnector" > > > > > > > > > > > > > > > > or > > > > > > > > > > > > > > > > > "FileStreamSink" instead of > > > > > > > > > > > > > > > > > > > > > > > > "org.apache.kafka.connect.file.FileStreamSinkConnector")? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 23, 2021 at 12:07 PM Mickael > > > Maison < > > > > > > > > > > > > > > > > mickael.mai...@gmail.com> > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks all for the feedback! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris, > > > > > > > > > > > > > > > > > > I agree that fixing the current endpoint > > > helps > > > > a > > > > > > > lot. Thanks for > > > > > > > > > > > > > > > > > > raising these JIRAs and submitting a PR! > > > > > > > > > > > > > > > > > > However thinking about the issue further, I > > > > decided > > > > > > > to expand the > > > > > > > > > > > > > > > > > > scope of the KIP to cover all user-visible > > > > plugins. > > > > > > > > > > > > > > > > > > In practice, users want to know about all > > > > available > > > > > > > plugins not > > > > > > > > > > > > > > only > > > > > > > > > > > > > > > > > > connectors. This includes transformations, > > > > > > > converters, > > > > > > > > > > > > > > > > > > header_converters and predicates. As we also > > > > want to > > > > > > > retrieve > > > > > > > > > > > > > > > > > > configdef for these too, I think it makes > > > > sense to > > > > > > > introduce a new > > > > > > > > > > > > > > > > > > endpoint to do so. Alongside we obviously > > > need > > > > a new > > > > > > > endpoint for > > > > > > > > > > > > > > > > > > listing all plugins. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Gunnar, > > > > > > > > > > > > > > > > > > I took a look at exposing valid values via > > > the > > > > API. > > > > > > > I think the > > > > > > > > > > > > > > issue > > > > > > > > > > > > > > > > > > is that Validators don't expose a way to > > > > retrieve > > > > > > > valid values. > > > > > > > > > > > > > > > > > > Changing validators will have an impact on > > > all > > > > > > > components so I'd > > > > > > > > > > > > > > > > > > prefer to address this requirement in a > > > > separate > > > > > > > KIP. I agree this > > > > > > > > > > > > > > > > > > would be an interesting improvement and I'd > > > > happy to > > > > > > > write a KIP > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > it too. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have updated the KIP accordingly. Let me > > > > know if > > > > > > > you have further > > > > > > > > > > > > > > > > > > feedback. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > Mickael > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 16, 2021 at 9:31 PM Gunnar > > > Morling > > > > > > > > > > > > > > > > > > <gunnar.morl...@googlemail.com.invalid> > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm +1 for adding a GET endpoint for > > > > obtaining > > > > > > > config > > > > > > > > > > > > > > definitions. It > > > > > > > > > > > > > > > > > > > always felt odd to me that one has to > > > > > > > > > > > > > > > > > > > issue > > > > a PUT > > > > > > > for that > > > > > > > > > > > > > > purpose. > > > > > > > > > > > > > > > > If > > > > > > > > > > > > > > > > > > > nothing else, it'd be better in terms of > > > > > > > discoverability of the > > > > > > > > > > > > > > KC > > > > > > > > > > > > > > > > REST > > > > > > > > > > > > > > > > > > API. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > One additional feature request I'd have is > > > to > > > > > > > expose the valid > > > > > > > > > > > > > > enum > > > > > > > > > > > > > > > > > > > constants for enum-typed options. That'll > > > > help to > > > > > > > display the > > > > > > > > > > > > > > values > > > > > > > > > > > > > > > > in a > > > > > > > > > > > > > > > > > > > drop-down or via radio buttons in a UI, > > > give > > > > us > > > > > > > tab completion in > > > > > > > > > > > > > > > > kcctl, > > > > > > > > > > > > > > > > > > > etc. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --Gunnar > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Am Di., 16. Nov. 2021 um 16:31 Uhr schrieb > > > > Chris > > > > > > > Egerton > > > > > > > > > > > > > > > > > > > <chr...@confluent.io.invalid>: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Viktor, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It sounds like there are three major > > > > points here > > > > > > > in favor of a > > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > GET > > > > > > > > > > > > > > > > > > > > endpoint for connector config defs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. You cannot issue a blank ("dummy") > > > > request > > > > > > > for sink > > > > > > > > > > > > > > connectors > > > > > > > > > > > > > > > > > > because a > > > > > > > > > > > > > > > > > > > > topic list/topic regex has to be > > > > > > > > > > > > > > > > > > > > supplied > > > > > > > (otherwise the PUT > > > > > > > > > > > > > > > > endpoint > > > > > > > > > > > > > > > > > > > > returns a 500 response) > > > > > > > > > > > > > > > > > > > > 2. A dummy request still triggers custom > > > > > > > validations by the > > > > > > > > > > > > > > > > connector, > > > > > > > > > > > > > > > > > > > > which may be best to avoid if we know > > > > > > > > > > > > > > > > > > > > for > > > > sure > > > > > > > that the config > > > > > > > > > > > > > > > > isn't > > > > > > > > > > > > > > > > > > worth > > > > > > > > > > > > > > > > > > > > validating yet > > > > > > > > > > > > > > > > > > > > 3. It's more ergonomic and intuitive to > > > be > > > > able > > > > > > > to issue a GET > > > > > > > > > > > > > > > > request > > > > > > > > > > > > > > > > > > > > without having to give a dummy connector > > > > config > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > With regards to 1, this is actually a > > > > > > > > > > > > > > > > > > > > bug > > > > in > > > > > > > Connect ( > > > > > > > > > > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/KAFKA-13327) with a fix > > > > > > > > > > > > > > > > already > > > > > > > > > > > > > > > > > > > > implemented and awaiting committer > > > review ( > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/pull/11369). > > > > I > > > > > > > think it'd be > > > > > > > > > > > > > > > > better to > > > > > > > > > > > > > > > > > > > > focus on fixing this bug in general > > > > instead of > > > > > > > implementing a > > > > > > > > > > > > > > new > > > > > > > > > > > > > > > > REST > > > > > > > > > > > > > > > > > > > > endpoint in order to allow people to > > > > > > > > > > > > > > > > > > > > work > > > > around > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > With regards to 2, this is technically > > > > possible > > > > > > > but I'm unsure > > > > > > > > > > > > > > > > it'd be > > > > > > > > > > > > > > > > > > too > > > > > > > > > > > > > > > > > > > > common out in the wild given that most > > > > > > > validations that could > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > expensive > > > > > > > > > > > > > > > > > > > > would involve things like connecting to > > > > > > > > > > > > > > > > > > > > a > > > > > > > database, checking > > > > > > > > > > > > > > if a > > > > > > > > > > > > > > > > cloud > > > > > > > > > > > > > > > > > > > > storage bucket exists, etc., none of > > > which > > > > are > > > > > > > possible without > > > > > > > > > > > > > > > > some > > > > > > > > > > > > > > > > > > > > configuration properties from the user > > > (db > > > > > > > hostname, bucket > > > > > > > > > > > > > > name, > > > > > > > > > > > > > > > > > > etc.). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > With regards to 3, I do agree that it'd > > > be > > > > > > > easier for people > > > > > > > > > > > > > > > > designing > > > > > > > > > > > > > > > > > > UIs > > > > > > > > > > > > > > > > > > > > to have a GET API to work against. I'm > > > > just not > > > > > > > sure it's > > > > > > > > > > > > > > worth the > > > > > > > > > > > > > > > > > > > > additional implementation, testing, and > > > > > > > maintenance burden. If > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > were > > > > > > > > > > > > > > > > > > > > possible to issue a PUT request without > > > > > > > unexpected 500s for > > > > > > > > > > > > > > invalid > > > > > > > > > > > > > > > > > > > > configs, would that suffice? AFAICT it'd > > > > > > > basically be as > > > > > > > > > > > > > > simple as > > > > > > > > > > > > > > > > > > issuing > > > > > > > > > > > > > > > > > > > > a PUT request with a dummy body > > > consisting > > > > of > > > > > > > nothing except > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > > > > > class (which at this point we might even > > > > make > > > > > > > unnecessary and > > > > > > > > > > > > > > just > > > > > > > > > > > > > > > > > > > > automatically replace with the connector > > > > class > > > > > > > from the URL) > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > then > > > > > > > > > > > > > > > > > > > > filtering the response to just grab the > > > > > > > "definition" field of > > > > > > > > > > > > > > each > > > > > > > > > > > > > > > > > > element > > > > > > > > > > > > > > > > > > > > in the "configs" array in the response. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 16, 2021 at 9:52 AM Viktor > > > > > > > Somogyi-Vass < > > > > > > > > > > > > > > > > > > > > viktorsomo...@gmail.com> > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Folks, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I too think this would be a very > > > > > > > > > > > > > > > > > > > > > useful > > > > > > > feature. Some of our > > > > > > > > > > > > > > > > > > management > > > > > > > > > > > > > > > > > > > > > applications would provide a wizard > > > > > > > > > > > > > > > > > > > > > for > > > > > > > creating connectors. > > > > > > > > > > > > > > In > > > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > > > scenario the user basically would fill > > > > out a > > > > > > > sample > > > > > > > > > > > > > > configuration > > > > > > > > > > > > > > > > > > > > generated > > > > > > > > > > > > > > > > > > > > > by the UI which would send it back to > > > > Connect > > > > > > > for validation > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > eventually > > > > > > > > > > > > > > > > > > > > > create a new connector. The first part > > > > of this > > > > > > > workflow can > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > enhanced > > > > > > > > > > > > > > > > > > > > if > > > > > > > > > > > > > > > > > > > > > we had an API that can return the > > > > > > > configuration definition > > > > > > > > > > > > > > of the > > > > > > > > > > > > > > > > > > given > > > > > > > > > > > > > > > > > > > > > type of connector as the UI > > > > > > > > > > > > > > > > > > > > > application > > > > would > > > > > > > be able to > > > > > > > > > > > > > > > > generate a > > > > > > > > > > > > > > > > > > > > sample > > > > > > > > > > > > > > > > > > > > > for the user based on that (nicely > > > drawn > > > > > > > diagram: > > > > > > > > > > > > > > > > > > > > > https://imgur.com/a/7S1Xwm5). > > > > > > > > > > > > > > > > > > > > > The > > > > > > > connector-plugins/{connectorType}/config/validate API > > > > > > > > > > > > > > > > essentially > > > > > > > > > > > > > > > > > > > > works > > > > > > > > > > > > > > > > > > > > > and returns the data that we need, > > > > however it > > > > > > > is a HTTP PUT > > > > > > > > > > > > > > API > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > is a > > > > > > > > > > > > > > > > > > > > > bit unintuitive for a fetch-like > > > > functionality > > > > > > > and also > > > > > > > > > > > > > > > > functionally > > > > > > > > > > > > > > > > > > > > > different as it validates the given > > > > (dummy) > > > > > > > request. In case > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > sink > > > > > > > > > > > > > > > > > > > > > connectors one would need to also > > > > provide a > > > > > > > topic name. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > A suggestion for the KIP: I think it > > > can > > > > be > > > > > > > useful to return > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > config > > > > > > > > > > > > > > > > > > > > > groups and the connector class' name > > > > similarly > > > > > > > to the > > > > > > > > > > > > > > validate > > > > > > > > > > > > > > > > API > > > > > > > > > > > > > > > > > > just > > > > > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > > > > > case any frontend needs them (and also > > > > the > > > > > > > response would be > > > > > > > > > > > > > > more > > > > > > > > > > > > > > > > > > like > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > validate API but simpler). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Viktor > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 20, 2021 at 4:51 PM Ryanne > > > > Dolan < > > > > > > > > > > > > > > > > ryannedo...@gmail.com> > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think it'd be worth adding a GET > > > > version, > > > > > > > fwiw. Could be > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > same > > > > > > > > > > > > > > > > > > > > > handler > > > > > > > > > > > > > > > > > > > > > > with just a different spelling > > > > > > > > > > > > > > > > > > > > > > maybe. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 20, 2021, 7:44 AM > > > > > > > > > > > > > > > > > > > > > > Mickael > > > > Maison > > > > > > > < > > > > > > > > > > > > > > > > > > mickael.mai...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You're right, you can achieve the > > > > same > > > > > > > functionality > > > > > > > > > > > > > > using > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > existing validate endpoint. > > > > > > > > > > > > > > > > > > > > > > > In my mind it was only for > > > > validation once > > > > > > > you have > > > > > > > > > > > > > > build a > > > > > > > > > > > > > > > > > > > > > > > configuration but when used with > > > > > > > > > > > > > > > > > > > > > > > an > > > > empty > > > > > > > configuration, > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > basically > > > > > > > > > > > > > > > > > > > > > > > serves the same purpose as the > > > > proposed > > > > > > > new endpoint. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think it's a bit easier to use a > > > > GET > > > > > > > endpoint but I > > > > > > > > > > > > > > don't > > > > > > > > > > > > > > > > > > think it > > > > > > > > > > > > > > > > > > > > > > > really warrants a different > > > endpoint. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 19, 2021 at 2:56 PM > > > Chris > > > > > > > Egerton > > > > > > > > > > > > > > > > > > > > > > > <chr...@confluent.io.invalid> > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Mickael, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm wondering about the use case > > > > here. > > > > > > > The motivation > > > > > > > > > > > > > > > > section > > > > > > > > > > > > > > > > > > > > states > > > > > > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > > > > "Connect does not provide a way > > > to > > > > see > > > > > > > what > > > > > > > > > > > > > > configurations > > > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > > > > > > > > > requires. Instead users have to > > > go > > > > look > > > > > > > at the > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > > > > > > documentation > > > > > > > > > > > > > > > > > > > > > > or > > > > > > > > > > > > > > > > > > > > > > > > in the worst case, look directly > > > > at the > > > > > > > connector > > > > > > > > > > > > > > source > > > > > > > > > > > > > > > > > > code.", > > > > > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > > > > with this KIP, "users will be > > > able > > > > to > > > > > > > discover the > > > > > > > > > > > > > > required > > > > > > > > > > > > > > > > > > > > > > > configurations > > > > > > > > > > > > > > > > > > > > > > > > for connectors installed in a > > > > Connect > > > > > > > cluster" and > > > > > > > > > > > > > > "tools > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > > > able > > > > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > > generate wizards for configuring > > > > and > > > > > > > starting > > > > > > > > > > > > > > connectors". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Does the existing "PUT > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > /connector-plugins/{connector-type}/config/validate" > > > > > > > > > > > > > > > > > > > > > > > > endpoint not address these > > > points? > > > > What > > > > > > > will the > > > > > > > > > > > > > > > > newly-proposed > > > > > > > > > > > > > > > > > > > > > > endpoint > > > > > > > > > > > > > > > > > > > > > > > > allow users to do that they will > > > > not > > > > > > > already be able > > > > > > > > > > > > > > to do > > > > > > > > > > > > > > > > > > with the > > > > > > > > > > > > > > > > > > > > > > > > existing endpoint? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Aug 19, 2021 at 9:20 AM > > > > Mickael > > > > > > > Maison < > > > > > > > > > > > > > > > > > > > > > > mickael.mai...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I've created KIP-769 to expose > > > > > > > connector > > > > > > > > > > > > > > configuration > > > > > > > > > > > > > > > > > > > > definitions > > > > > > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > > > > > > > > > the Connect API > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-769%3A+Connect+API+to+retrieve+connector+configuration+definitions > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please take a look and let me > > > > know if > > > > > > > you have any > > > > > > > > > > > > > > > > feedback. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
