Thanks, Chris and Mickael. The use case does make sense, and I can see some value in it. We do have precedent for some administrative endpoints, including KIP-495 [1] that adds the resource `admin/loggers`. Since the use case persona is a Connect cluster administrator, perhaps we could move this `/worker-plugins` endpoint under this `/admin/` path to help distinguish this and to centralize at least these endpoints in case they need to be secured either by the admin (e.g., via a proxy or gateway) or as a future KIP that makes them configurable.
I'm still not sure, though. I accept that the effort to add the worker-plugins resource is not significant. My main concern is about the additional surface area and whether the stated use case is _valuable enough_ to warrant this additional surface area. The connector, converter, and SMT plugins are all designed to be used via the API by clients, so exposing these available plugins makes sense. But the worker plugins are only usable via a worker configuration change, and exposing the available worker plugins may provide more information via API to malicious users that wouldn't otherwise without direct access to the machines. Best regards, Randall [1] https://cwiki.apache.org/confluence/display/KAFKA/KIP-495%3A+Dynamically+Adjust+Log+Levels+in+Connect On Fri, Dec 3, 2021 at 10:52 AM Chris Egerton <chr...@confluent.io.invalid> wrote: > > Hi Mickael and Randall, > > I think there's a decent use case for exposing worker plugins. If a cluster > administrator wants to enable a worker plugin on their cluster, it may make > things easier if they can verify ahead of time that that plugin is > available on every worker in the cluster (and possibly that the same > version is present on every worker). The current alternative to this would > require sifting through worker logs. > > I don't think the additional workload to expose worker plugins is > significant (it should largely mirror the same logic we'll use to expose > connector plugins), but if there's serious objection to including that > feature in this KIP, we could probably live without it now that we've made > sure that the current proposal doesn't block us from adding it in a > reasonable way later on down the road. > > Cheers, > > Chris > > On Fri, Dec 3, 2021 at 11: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 > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
