Hi Snehashis, Sorry for the late reply.
> Heterogeneous dependencies in a multi cluster deployment is highly discouraged Right, this remains unchanged in this KIP. > Let's add the version information for both connector and tasks in the connector status itself > once we add these two additions to the KIP (LMK if you want me to take that up). Could you make these additions? I'm interested to see if we can include this in 4.0. Thanks, Greg On Tue, Jul 2, 2024 at 2:52 AM Snehashis <snehashisp1...@gmail.com> wrote: > Hi Greg, > > Thanks for taking a look at this, to conclude on the two points above. > > 1. I'm okay with the status quo of leaving the dependency management of > plugins to systems outside of the connect runtime as it is now. Given that > the dependencies are homogenous across a connect cluster, it should ensure > that task and connector versions are uniform across a deployment, after it > has stabilised post an upgrade operation. Heterogeneous dependencies in a > multi cluster deployment is highly discouraged and we should point out this > can lead to inconsistent behaviour with connectors and even validations. > > 2. Let's add the version information for both connector and tasks in the > connector status itself, I don't see another endpoint being required for > this. > > I don't have any further points, so if you are okay we can put this to a > vote, once we add these two additions to the KIP (LMK if you want me to > take that up). > > Regards > Snehashis > > On Tue, Jul 2, 2024 at 12:08 AM Greg Harris <greg.har...@aiven.io.invalid> > wrote: > > > Hey Snehashis, > > > > Sorry for the late reply, and thanks for helping close out the > discussion. > > > > > Note that if my assumptions are correct then > > > this can happen with the existing framework as well, or is there some > > > safeguard from this happening? > > > > Currently, if a cluster has a heterogeneous plugin installation, each > > worker may have a different definition of "latest". If you call to > > validate a configuration, it will validate against whatever latest > version > > is present locally. > > With this KIP, that behavior will continue when the version isn't > > specified, when the version is a soft requirement, or when the version > is a > > range. For users that do not want to tolerate their tasks having > > heterogeneous versions, the single-version hard requirements are there, > but > > come at the cost of micromanagement of upgrades. > > Perhaps there is some room in the future for a connector configured with > a > > range to "pin" the version that all of its tasks should use. Or > > version-aware-scheduling can schedule connectors and tasks to workers > that > > will resolve the same versions. The important primitive is being able to > > enforce which version is being used, everything else looks more like a > > convenience feature to me. > > > > > So far, we could have pointed to the > > > misconfigured cluster configuration and somewhat differ this problem to > > > something outside of connect runtime. > > > > I think this will still be the case. Even with this feature, we will > still > > recommend homogeneous clusters that have the same set of plugins > installed > > on every worker. Version-aware scheduling is a rejected alternative, and > > your example for performing validation requests on an arbitrary worker > is a > > great example of a complication for heterogeneous clusters that we're not > > ready to solve. > > > > > We can introduce > > > a new path under this for version (/connector/connector-name/version), > > but > > > perhaps adding this as part of the status is a valid alternative. > > > > I think either is fine. Adding a new field to the existing endpoints is > > fine, and shouldn't require special compatibility such as a query > > parameter. > > > > > Also, to go further I think > > > version information for tasks could also be available, > > > > I completely agree. The version of each connector and each task should be > > visible individually, since they may be different during a rolling > upgrade. > > > > Thanks! > > Greg > > > > On Tue, Jun 18, 2024 at 5:18 AM Snehashis <snehashisp1...@gmail.com> > > wrote: > > > > > Hi Greg, Chris > > > > > > Thanks for the in-depth discussion, I have a couple of discussion > points > > > and would like your thoughts on this. > > > > > > 1) One concern I have with the new addition of 'soft' and 'hard' > version > > > requirements is that there could be a mismatch in the plugin version > that > > > two different tasks are running, if a soft requirement is provided and > > the > > > nodes a multi cluster deployment are not in sync w.r.t the plugin > > versions > > > that they are configured with. Note that if my assumptions are correct > > then > > > this can happen with the existing framework as well, or is there some > > > safeguard from this happening? So far, we could have pointed to the > > > misconfigured cluster configuration and somewhat differ this problem to > > > something outside of connect runtime. With this feature in place > perhaps > > > the expectation is more on connect to not be running with such > > > inconsistency, especially if a connector version is specified. This is > > also > > > a problem with validation if different cluster have different > > > configurations, as IIRC validations are local to the worker which > > receives > > > the rest call for validate. So, we might be validating with a certain > > > version which is different from the one that will be used to create > > > connector and tasks. Again, this is likely how the current state is, > but > > > perhaps such inconsistencies warrant a deeper look with the addition of > > > this feature. The problems associated with them can be somewhat > insidious > > > and hard to diagnose. > > > > > > 2) There was some discussion on the need for a new REST endpoint to > > provide > > > information on the versions of running connectors, and I think adding > > this > > > information via REST is a valuable addition. The way I see it the > version > > > is an intrinsic property of an instance of a running connector and > hence > > > this should be part of the set of APIs under > /connector/<connector-name> > > > (also the /connectors API should also have this information as it is an > > > amalgamation of all the individual connector information). We can > > introduce > > > a new path under this for version (/connector/connector-name/version), > > but > > > perhaps adding this as part of the status is a valid alternative. This > is > > > mentioned as a rejected alternative right now. Also, to go further I > > think > > > version information for tasks could also be available, especially if we > > > choose to not address the pitfalls discussed in my point 1), this will > > > at-least provide admins a quick and easy way to determine if such and > > > inconsistent state exist in any of the connectors. > > > > > > Thanks again for reviving my original KIP and working to improve it. > > > Looking forward to your thoughts on the points mentioned above. > > > Regards > > > Snehashis > > > > > > > > > On Wed, May 29, 2024 at 9:59 PM Chris Egerton <chr...@aiven.io.invalid > > > > > wrote: > > > > > > > Hi Greg, > > > > > > > > First, an apology! I mistakenly assumed that each plugin appeared > only > > > once > > > > in the responses from GET /connector-plugins?connectorsOnly=false. > > Thank > > > > you for correcting me and pointing out that all versions of each > plugin > > > > appear in that response, which does indeed satisfy my desire for > users > > to > > > > discover this information in at most two REST requests (and in fact, > > does > > > > it in only one)! > > > > > > > > And secondly, with the revelation about recommenders, I agree that > it's > > > > best to leave the "version" property out of the lists of properties > > > > returned from the GET /connector-plugins/<plugin>/config endpoint. > > > > > > > > With those two points settled, I think the only unresolved item is > the > > > > small change to version parsing added to the KIP (where raw version > > > numbers > > > > are treated as an exact match, instead of a best-effort match with a > > > > fallback on the default version). If the KIP is updated with that > then > > > I'd > > > > be ready to vote on it. > > > > > > > > Cheers, > > > > > > > > Chris > > > > > > > > On Wed, May 29, 2024 at 12:00 PM Greg Harris > > > <greg.har...@aiven.io.invalid > > > > > > > > > wrote: > > > > > > > > > Hey Chris, > > > > > > > > > > Thanks for your thoughts. > > > > > > > > > > > Won't it still only expose the > > > > > > latest version for each plugin, instead of the range of versions > > > > > available? > > > > > > > > > > Here is a snippet of the current output of the GET > > > > > /connector-plugins?connectorsOnly=false endpoint, after I installed > > two > > > > > versions of the debezium PostgresConnector: > > > > > > > > > > { > > > > > "class": "io.debezium.connector.postgresql.PostgresConnector", > > > > > "type": "source", > > > > > "version": "2.0.1.Final" > > > > > }, > > > > > { > > > > > "class": "io.debezium.connector.postgresql.PostgresConnector", > > > > > "type": "source", > > > > > "version": "2.6.1.Final" > > > > > }, > > > > > > > > > > I think this satisfies your requirement to learn about all plugins > > and > > > > all > > > > > versions in two or fewer REST calls. > > > > > > > > > > I tried to get an example of the output of `/config` by hardcoding > > the > > > > > Recommender, and realized that Recommenders aren't executed on the > > > > > `/config` endpoint at all: only during validation, when a > > configuration > > > > is > > > > > actually present. > > > > > And this led me to discover that the `/config` endpoint returns a > > > > > List<ConfigKeyInfo>, and ConfigKeyInfo does not contain a > > > > recommendedValues > > > > > field. The ConfigValue field is the object which contains > > > > > recommendedValues, and it is only generated during validation. > > > > > I think it's out of scope to start calling recommenders on empty > > > configs > > > > > that might throw exceptions, changing the existing REST entities, > or > > > > > changing the core ConfigDef implementation. > > > > > Someone could add this functionality later, I don't think it's > > > necessary > > > > > here. > > > > > > > > > > Then the question is: should "version" without recommenders appear > in > > > > > non-connector plugins? I think I'd rather be consistent with > > > "predicate" > > > > > and "negate" on release, and let a later improvement add them. > > > > > > > > > > Thanks, > > > > > Greg > > > > > > > > > > On Wed, May 29, 2024 at 8:06 AM Chris Egerton > > <chr...@aiven.io.invalid > > > > > > > > > wrote: > > > > > > > > > > > Hi Greg, > > > > > > > > > > > > I'm confused about the behavior for GET > > > > > > /connector-plugins?connectorsOnly=false. Won't it still only > expose > > > the > > > > > > latest version for each plugin, instead of the range of versions > > > > > available? > > > > > > > > > > > > I'm hoping we can provide a flow where people need at most two > REST > > > > calls > > > > > > to discover 1) the complete set of plugins available on the > worker > > > > (which > > > > > > is already possible with the endpoint under discussion) and 2) > the > > > set > > > > of > > > > > > versions available for a specific plugin on the worker (which > > doesn't > > > > > > appear to be possible, at least for some plugin types). This > > wouldn't > > > > > > require any out-of-band knowledge and would be valuable for > > connector > > > > > users > > > > > > (who may want to, for example, know what their options are when > > > > > considering > > > > > > a plugin upgrade) and cluster administrators (who could use it > as a > > > > > sanity > > > > > > check for the setup of their Kafka Connect clusters without > having > > to > > > > > pore > > > > > > over log files). > > > > > > > > > > > > As far as modifying the content of the GET > > > > > > /connector-plugins/<plugin>/config endpoint to include a > "version" > > > > > property > > > > > > goes, I think your point about returning that property from > > requests > > > to > > > > > > that endpoint that include a version query parameter is salient, > > but > > > it > > > > > > also unfortunately applies to all types of plugin. I don't think > it > > > > > should > > > > > > completely disqualify that option. I also wasn't imagining adding > > > > > anything > > > > > > besides that single property to that endpoint, so no new > > "predicate" > > > > > > property for SMTs, no new "negate" property for predicates, and > no > > > new > > > > > > "type" property for either. I'm not necessarily opposed to adding > > > > those, > > > > > > but it can be done without being pulled into the scope of this > KIP. > > > > > > > > > > > > So to summarize how I imagine people might use the REST API after > > > this > > > > > KIP: > > > > > > > > > > > > 1. Discover the set of plugins on the worker via GET > > > > > > /connector-plugins?connectorsOnly=false > > > > > > 2. For any of those plugins, discover the set of available > versions > > > by > > > > > > examining the recommended values for the "version" property after > > > > hitting > > > > > > the GET GET /connector-plugins/<plugin>/config endpoint > > > > > > > > > > > > I don't think this is necessarily optimal since plucking the > > > "version" > > > > > > property out of the response from that endpoint might be a PITA > for > > > ad > > > > > hoc > > > > > > queries via, e.g., curl, but I think it'd be enough for this KIP. > > > Still > > > > > > open to other options though, and if I've misunderstood anything > in > > > the > > > > > > proposal, please let me know! > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Chris > > > > > > > > > > > > On Wed, May 22, 2024 at 4:05 PM Greg Harris > > > > <greg.har...@aiven.io.invalid > > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hey Chris, > > > > > > > > > > > > > > Thanks for your comments, and I'm glad that it seems like we're > > > > > > > aligning on the vision here. > > > > > > > > > > > > > > > An > > > > > > > > alternative could be to change existing behavior to fail fast > > on > > > > any > > > > > > > > invalid default converter configuration instead of just for > > > invalid > > > > > > > > versions > > > > > > > > > > > > > > I suppose if this is landing in 4.0, we have the opportunity to > > > break > > > > > > > compatibility and strictly validate the worker class configs, > and > > > > > > > could have a new consistent behavior instead of inconsistent > but > > > > > > > backwards-compatible behavior. > > > > > > > > > > > > > > > no other part of the KIP requires this change. > > > > > > > > > > > > > > This is correct, and a compelling argument. I'm fine leaving > the > > > > > > > strict worker validation off of this configuration to > potentially > > > be > > > > > > > added later. If a KIP was raised to perform strict validation > of > > > the > > > > > > > worker config, it would include the version config, and can > > address > > > > > > > backwards compatibility for both configs together. > > > > > > > > > > > > > > > RE exposing the version property in the > > > > > > > /connector-plugins/<plugin>/config > > > > > > > > endpoint, the behavior is inconsistent across plugin types. > > > > > > > > > > > > > > Yes it is, and that started in a bugfix, once people started > > using > > > > > > > this endpoint: > https://issues.apache.org/jira/browse/KAFKA-14843 > > > . I > > > > > > > wasn't planning on converging the behavior in this KIP, as I > > > > > > > considered it out-of-scope, and was going to follow the current > > > > > > > behavior. To summarize: > > > > > > > > > > > > > > GET /connector-plugins/<connector>/config will emit: > > > > > > > * connector.class (already implemented) > > > > > > > * connector.version (new) > > > > > > > * key.converter (already implemented) > > > > > > > * key.converter.version (new) > > > > > > > * value.converter (already implemented) > > > > > > > * value.converter.version (new) > > > > > > > * header.converter (already implemented) > > > > > > > * header.converter.version (new) > > > > > > > But will NOT emit: > > > > > > > * transforms.<alias>.type > > > > > > > * transforms.<alias>.version > > > > > > > * transforms.<alias>.predicate > > > > > > > * predicates.<alias>.type > > > > > > > * predicates.<alias>.version > > > > > > > * predicates.<alias>.negate > > > > > > > > > > > > > > GET /connector-plugins/<converter>/config will NOT emit: > > > > > > > * "" (there's not even a ".class" prefix!) > > > > > > > * version > > > > > > > > > > > > > > GET /connector-plugins/<transform>/config will NOT emit: > > > > > > > * type > > > > > > > * version > > > > > > > * predicate > > > > > > > > > > > > > > GET /connector-plugins/<predicate>/config will NOT emit: > > > > > > > * type > > > > > > > * version > > > > > > > * negate > > > > > > > > > > > > > > Do you want the converter, transform, and predicate endpoints > > > > changed? > > > > > > > Do you want just "version", or do you want all of the prefixed > > > > configs > > > > > > > including "type", "predicate" and "negate"? How would you want > to > > > > > > > handle the converters? > > > > > > > And when I say the configs are "not part of the plugin config > > > itself" > > > > > > > I mean that saying that GET > > > > > > > /connector-plugins/Flatten$Key/config?version=3.8.0 has a > > "version" > > > > > > > config that must be "3.8.0" is a little bit nonsense, as the > > > version > > > > > > > is already specified. > > > > > > > > > > > > > > > IMO > > > > > > > > it's worth including this information somewhere directly > > > accessible > > > > > > > without > > > > > > > > having to provide a full connector config. FWIW I'd be fine > > with > > > > GET > > > > > > > > /connector-plugins/<plugin>/versions as a first-class > endpoint > > > > > > > > > > > > > > You don't have to provide a configuration to call GET > > > > > > > /connector-plugins?connectorsOnly=false , is that endpoint not > > > close > > > > > > > enough to what you have in mind? See also the Rejected > > Alternative > > > > > > > "Adding new REST API endpoints" > > > > > > > > > > > > > > If you're calling /connector-plugins/<transform>/config, you > know > > > the > > > > > > > name of a plugin right? That either comes from out-of-band > > > knowledge, > > > > > > > validating a connector config, or calling GET > > > > > > > /connector-plugins?connectorsOnly=false. > > > > > > > * If you have out-of-band knowledge of plugin classes, perhaps > > you > > > > > > > have out-of-band knowledge of versions too. > > > > > > > * If you've just validated a connector config, there should be > an > > > > > > > accompanying "version" field there with an accurate default > value > > > and > > > > > > > recommenders. > > > > > > > * If you've called GET /connector-plugins?connectorsOnly=false, > > > that > > > > > > > endpoint includes version information. > > > > > > > > > > > > > > Thanks, > > > > > > > Greg > > > > > > > > > > > > > > On Wed, May 22, 2024 at 11:05 AM Chris Egerton > > > > <chr...@aiven.io.invalid > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Greg, > > > > > > > > > > > > > > > > Hope you had a nice weekend! Gonna try to keep things > concise. > > > > > > > > > > > > > > > > Concluded points: > > > > > > > > > > > > > > > > RE version recommenders, I agree it's likely that > programmatic > > > UIs > > > > > will > > > > > > > > already be able to handle dynamic configuration definitions, > > and > > > > the > > > > > > > detail > > > > > > > > about SMTs is a great point. I still anticipate some > > awkwardness > > > > with > > > > > > > > connector versions, though: if the latest version supports > some > > > new > > > > > > > > properties, then a user switches to an earlier version, a UI > > may > > > > > > respond > > > > > > > by > > > > > > > > wiping values for these properties. I guess we can bite the > > > bullet, > > > > > > > though. > > > > > > > > > > > > > > > > RE double-dinging during preflight validation for invalid > > > > versions, I > > > > > > > like > > > > > > > > the analogy with login credentials. I'm convinced that the > > > proposal > > > > > in > > > > > > > the > > > > > > > > KIP is best 👍 > > > > > > > > > > > > > > > > Continued points: > > > > > > > > > > > > > > > > RE failing on worker startup, sorry, I should be clearer: > there > > > is > > > > no > > > > > > > _new_ > > > > > > > > justification for it that doesn't also apply to existing > > > behavior. > > > > We > > > > > > > > shouldn't diverge from existing behavior solely for this new > > > case. > > > > An > > > > > > > > alternative could be to change existing behavior to fail fast > > on > > > > any > > > > > > > > invalid default converter configuration instead of just for > > > invalid > > > > > > > > versions, but I'd vote to just stick to existing behavior and > > not > > > > > > > > complicate things, especially since no other part of the KIP > > > > requires > > > > > > > this > > > > > > > > change. > > > > > > > > > > > > > > > > RE exposing the version property in the > > > > > > > /connector-plugins/<plugin>/config > > > > > > > > endpoint, the behavior is inconsistent across plugin types. > > > Hitting > > > > > the > > > > > > > > endpoint for the FileStreamSinkConnector on version 3.7.0 > > yields > > > a > > > > > > > response > > > > > > > > that includes, among other things, the "topics", > > "topics.regex", > > > > and > > > > > > > > "errors.tolerance" properties. I see that we don't do this > > > > everywhere > > > > > > > (the > > > > > > > > examples you cite for SMT and converter properties are > > accurate), > > > > but > > > > > > IMO > > > > > > > > it's worth including this information somewhere directly > > > accessible > > > > > > > without > > > > > > > > having to provide a full connector config. FWIW I'd be fine > > with > > > > GET > > > > > > > > /connector-plugins/<plugin>/versions as a first-class > endpoint > > > > either > > > > > > > > instead of or in addition to adding recommended values for > all > > > > plugin > > > > > > > > versions. > > > > > > > > > > > > > > > > Thanks for your continued work on this KIP, and with the > > progress > > > > > we're > > > > > > > > making I'm optimistic about its chances of appearing in > 4.0.0. > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > On Wed, May 15, 2024 at 1:22 PM Greg Harris > > > > > > <greg.har...@aiven.io.invalid > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hey Chris, > > > > > > > > > > > > > > > > > > Thanks for your quick follow up. > > > > > > > > > > > > > > > > > > > But this risk is already present with > > > > > > > > > > existing error cases, and I don't see anything that > > justifies > > > > > > > changing > > > > > > > > > > existing behavior with an invalid converter class, or > > > diverging > > > > > > from > > > > > > > it > > > > > > > > > in > > > > > > > > > > the case of invalid converter versions. > > > > > > > > > > > > > > > > > > The justification is to fail-fast, and prevent REST API > users > > > > from > > > > > > > > > receiving errors from bad configs that they didn't write, > or > > > > maybe > > > > > > > > > don't even know apply to them. > > > > > > > > > Up until recently errors in these configurations surfaced > as > > > > > failures > > > > > > > > > to create the connector, or failures to start, and you made > > > them > > > > > > > > > fail-fast during validation. I think this change is in the > > same > > > > > > > > > spirit, pulling the validation further forward and not > > letting > > > > > errors > > > > > > > > > lie dormant. > > > > > > > > > > > > > > > > > > And to call back to your original concern about > interrupting > > > > > > > > > connectors that explicitly provide these configurations and > > > don't > > > > > use > > > > > > > > > the worker configs: I expect that operators with a majority > > of > > > > > these > > > > > > > > > sorts of clients aren't going to be setting the worker > > .version > > > > > > > > > properties, because it would have no effect on the majority > > of > > > > > their > > > > > > > > > connectors. They would be able to rely on > > > backwards-compatibility > > > > > and > > > > > > > > > continue to ignore the class properties. > > > > > > > > > > > > > > > > > > > CLI > > > > > > > > > > and programmatic UI developers will want to develop their > > own > > > > > > tooling > > > > > > > > > > layers. > > > > > > > > > > > > > > > > > > This is a very compelling argument. Snehashis do you want > to > > > > figure > > > > > > > > > out a REST API design for this use-case? > > > > > > > > > > > > > > > > > > > Regarding the GET /connector-plugins/<plugin>/config > > > endpoint, > > > > I > > > > > > was > > > > > > > > > > thinking about the response for non-connector plugins, > > e.g., > > > > > > > > > > GET /connector-plugins/RegexRouter/config. Would a > > "version" > > > > > > property > > > > > > > > > > appear with recommended values? > > > > > > > > > > > > > > > > > > I intended for the ".version" property to be like other > > > framework > > > > > > > > > configs (".class", ".type", ".predicate", ".negate") where > > they > > > > are > > > > > > > > > inside the plugin namespace, but not part of the plugin > > config > > > > > > itself. > > > > > > > > > Perhaps we can deviate from those configs because it would > > aid > > > in > > > > > > > > > discovering other valid `GET > > > > > > > > > /connector-plugins/<plugin>/config?version=` calls, without > > > > calling > > > > > > > > > `GET /connector-plugins?connectorsOnly=false`. > > > > > > > > > I don't really feel strongly either way. > > > > > > > > > > > > > > > > > > > if a user changes the > > > > > > > > > > connector version in, e.g., a dropdown menu, then the UI > > > either > > > > > has > > > > > > > to > > > > > > > > > > re-fetch the ConfigDef for the new version, or risk > > operating > > > > on > > > > > > > stale > > > > > > > > > > information > > > > > > > > > > > > > > > > > > This is a really interesting situation, thanks for finding > > > that! > > > > > This > > > > > > > > > is already a footgun with transformations and predicates; > > Once > > > > you > > > > > > > > > fill out a transformation class (which includes a > > recommender) > > > > you > > > > > > > > > will then see all of the transformations configurations. > > > > > > > > > I think this means that UI developers should have already > > > > developed > > > > > > > > > the infrastructure for handling dynamic recommenders, or > else > > > > > they've > > > > > > > > > had this bug since KIP-66 in 2017. It may require some > manual > > > > > > > > > attention to roll-out support for the ".version" properties > > > > > > > > > specifically, but I don't think that should prevent us from > > > > > providing > > > > > > > > > this information in a natural place. > > > > > > > > > > > > > > > > > > > but will it be possible to configure > > > > > > > > > > a connector to use two instances of the same transform or > > > > > > predicate, > > > > > > > but > > > > > > > > > > with different versions for each? > > > > > > > > > > > > > > > > > > Yes, and this is included in the example in the KIP, > > > > > > > > > `transforms.flatten-latest` and `transforms.flatten-old`. > > This > > > > will > > > > > > > > > work in the natural way, with the two instances having a > > > > different > > > > > > > > > version if configured for that. If a user wants to pin the > > same > > > > > > > > > version of a plugin for every instance, they will need to > > > provide > > > > > the > > > > > > > > > version config multiple times and keep them in-sync > > themselves. > > > > > > > > > > > > > > > > > > > I would have expected the error > > > > > > > > > > to only be attributed to the version property, and for > the > > > > class > > > > > > > property > > > > > > > > > > to be reported as valid. > > > > > > > > > > > > > > > > > > I considered this, and then went back and changed it. To > > quote > > > > > > someone > > > > > > > > > else, this is to catch "misspelling cat as dog". For > example > > a > > > > user > > > > > > > > > types DogConverter and meant to type CatConverter, and then > > > > types a > > > > > > > > > version which is valid for CatConverter, conceptually the > > error > > > > is > > > > > in > > > > > > > > > the class name and not the version. > > > > > > > > > That's a very contrived scenario, but I think similar > > arguments > > > > are > > > > > > > > > used for attributing validation errors to > > > > endpoints/urls/hostnames > > > > > > > > > when the associated credentials are unable to log into the > > > remote > > > > > > > > > system. Did the user provide the correct testing > credentials, > > > but > > > > > > > > > accidentally typed the production endpoint? Even if the > > > > production > > > > > > > > > endpoint looks valid (it's a real hostname that is > reachable) > > > the > > > > > > > > > conceptual error is still in the hostname and should have > the > > > > error > > > > > > > > > attributed to it to draw the user's attention. > > > > > > > > > If that's not convincing, I think the alternative of only > > > > > attributing > > > > > > > > > version errors to the version property is also acceptable. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Greg > > > > > > > > > > > > > > > > > > On Wed, May 15, 2024 at 8:06 AM Chris Egerton > > > > > > <chr...@aiven.io.invalid > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Greg, > > > > > > > > > > > > > > > > > > > > Thanks for your responses! Continuations of existing > > > > discussions: > > > > > > > > > > > > > > > > > > > > Regarding crashing the worker on startup--yes, there is > > also > > > a > > > > > risk > > > > > > > to > > > > > > > > > > allowing it to join the cluster. But this risk is already > > > > present > > > > > > > with > > > > > > > > > > existing error cases, and I don't see anything that > > justifies > > > > > > > changing > > > > > > > > > > existing behavior with an invalid converter class, or > > > diverging > > > > > > from > > > > > > > it > > > > > > > > > in > > > > > > > > > > the case of invalid converter versions. I think we should > > > keep > > > > > this > > > > > > > > > simple > > > > > > > > > > and not do anything different for worker startup than we > do > > > > > > already. > > > > > > > > > > > > > > > > > > > > As far as REST API vs. metrics go--I think you're right > > that > > > > the > > > > > > > original > > > > > > > > > > version metrics were added as a "monitoring" detail. > > However, > > > > > this > > > > > > > was > > > > > > > > > back > > > > > > > > > > when plugin versions were managed solely by cluster > > > > > administrators. > > > > > > > With > > > > > > > > > > this KIP, connector users will be able to manage plugin > > > > versions, > > > > > > > and CLI > > > > > > > > > > and programmatic UI developers will want to develop their > > own > > > > > > tooling > > > > > > > > > > layers. I think focusing on the REST API as the primary > > > > interface > > > > > > for > > > > > > > > > this > > > > > > > > > > KIP would be best for these users. > > > > > > > > > > > > > > > > > > > > (All that said, I don't object to the metrics that are > > > proposed > > > > > in > > > > > > > the > > > > > > > > > KIP; > > > > > > > > > > I just think they make more sense in addition to new REST > > API > > > > > > > > > > functionality, as opposed to instead of it.) > > > > > > > > > > > > > > > > > > > > Regarding the GET /connector-plugins/<plugin>/config > > > endpoint, > > > > I > > > > > > was > > > > > > > > > > thinking about the response for non-connector plugins, > > e.g., > > > > > > > > > > GET /connector-plugins/RegexRouter/config. Would a > > "version" > > > > > > property > > > > > > > > > > appear with recommended values? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > And new thoughts: > > > > > > > > > > > > > > > > > > > > 1) Regarding the recommended values for > > "connector.version", > > > > this > > > > > > > might > > > > > > > > > be > > > > > > > > > > confusing since there could be differences between the > > > > ConfigDefs > > > > > > > for the > > > > > > > > > > latest version of the connector and prior versions. It > also > > > > makes > > > > > > the > > > > > > > > > flow > > > > > > > > > > a bit awkward for programmatic UI developers: if a user > > > changes > > > > > the > > > > > > > > > > connector version in, e.g., a dropdown menu, then the UI > > > either > > > > > has > > > > > > > to > > > > > > > > > > re-fetch the ConfigDef for the new version, or risk > > operating > > > > on > > > > > > > stale > > > > > > > > > > information. I'm starting to doubt that exposing the > range > > of > > > > > > > available > > > > > > > > > > versions via recommended values is the best way to > proceed, > > > > > instead > > > > > > > of a > > > > > > > > > > more explicit approach like GET > > > > > > > /connector-plugins/<plugin>/versions, or > > > > > > > > > > the "Adding new REST API endpoints" rejected alternative. > > > > > > > > > > > > > > > > > > > > 2) I know that this is a bit heinous, but will it be > > possible > > > > to > > > > > > > > > configure > > > > > > > > > > a connector to use two instances of the same transform or > > > > > > predicate, > > > > > > > but > > > > > > > > > > with different versions for each? (I don't think this is > > > worth > > > > > > > > > significant > > > > > > > > > > design/implementation effort, so if it would inflate > either > > > of > > > > > > those, > > > > > > > > > > please don't feel obligated to support it.) > > > > > > > > > > > > > > > > > > > > 3) Just out of curiosity, why double-ding during config > > > > > validation > > > > > > > if the > > > > > > > > > > version for a plugin class can't be found? I would have > > > > expected > > > > > > the > > > > > > > > > error > > > > > > > > > > to only be attributed to the version property, and for > the > > > > class > > > > > > > property > > > > > > > > > > to be reported as valid. > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > On Tue, May 14, 2024 at 1:11 PM Greg Harris > > > > > > > <greg.har...@aiven.io.invalid > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hey Chris, > > > > > > > > > > > > > > > > > > > > > > Thanks for your comments. I'm glad you like the > > > motivations, > > > > > > > Snehashis > > > > > > > > > > > wrote that part! > > > > > > > > > > > > > > > > > > > > > > > the configuration syntax for the most basic use case > of > > > > > > > > > > > > specifying a single desired version is pretty > > > > > counterintuitive. > > > > > > > > > > > > > > > > > > > > > > I agree, and the "soft" requirement scheme is > something I > > > > > wasn't > > > > > > > > > > > explicitly looking for, but would be inherited from the > > > > > library. > > > > > > > I'm > > > > > > > > > > > fine with eliminating the soft requirement semantics, > and > > > > > having > > > > > > > > > > > "1.0.0" behave the same as "[1.0.0]". I'm less inclined > > to > > > > > > include > > > > > > > > > > > "range" in the property name, or have two properties. > > > > > > > > > > > > > > > > > > > > > > > Failing startup is drastic and has the potential to > > > disrupt > > > > > the > > > > > > > > > > > > availability of connectors that would otherwise be > able > > > to > > > > > run > > > > > > > > > healthily > > > > > > > > > > > > because they were explicitly configured to use valid > > > > > converters > > > > > > > > > instead > > > > > > > > > > > of > > > > > > > > > > > > the worker defaults. > > > > > > > > > > > > > > > > > > > > > > I think this argument cuts both ways. If someone > > > > reconfigures a > > > > > > > worker > > > > > > > > > > > and adds an invalid ".version" string to the worker (or > > > > changes > > > > > > the > > > > > > > > > > > plugin.path to make it invalid), it would be permitted > to > > > > enter > > > > > > the > > > > > > > > > > > group, and accept work assignments. If those work > > > assignments > > > > > > used > > > > > > > > > > > these configurations, a set of tasks could transition > to > > > > FAILED > > > > > > and > > > > > > > > > > > not be able to recover, because they would be restarted > > > again > > > > > on > > > > > > > the > > > > > > > > > > > same worker. > > > > > > > > > > > > > > > > > > > > > > > Why are metrics utilized to report information about > > > plugin > > > > > > > versions > > > > > > > > > > > > utilized by connectors at runtime instead of > publishing > > > > this > > > > > > > info in > > > > > > > > > the > > > > > > > > > > > > REST API > > > > > > > > > > > > > > > > > > > > > > I was following the existing pattern for exposing > runtime > > > > > > versions > > > > > > > for > > > > > > > > > > > connectors, and it did seem like a "monitoring" > feature. > > If > > > > > that > > > > > > > > > > > approach is flawed and should be phased out, I think it > > > would > > > > > be > > > > > > a > > > > > > > > > > > good idea to reconsider the REST API rejected > > alternative. > > > > > > > > > > > We would need some additional design work to spec out > the > > > > REST > > > > > > API > > > > > > > > > > > interface, as I don't have anything in mind currently. > > > > > > > > > > > > > > > > > > > > > > > I'm unclear on whether or not it'll be possible to > see > > > this > > > > > > > > > information > > > > > > > > > > > via the > > > > > > > > > > > > GET /connector-plugins/<plugin>/config endpoint > > > > > > > > > > > > > > > > > > > > > > There is room in the API to add recommenders for > > > > > "key.converter", > > > > > > > > > > > "value.converter", and "header.converter", but not for > > > > > transforms > > > > > > > and > > > > > > > > > > > predicates, as they include aliases that depend on an > > > actual > > > > > > > > > > > configuration. We could explicitly say we're going to > do > > > > that, > > > > > or > > > > > > > do > > > > > > > > > > > whatever is convenient during the implementation phase, > > or > > > > > leave > > > > > > it > > > > > > > > > > > open to be improved later. > > > > > > > > > > > There will not be any recommenders for ".version" > > > properties > > > > in > > > > > > the > > > > > > > > > > > `/config` endpoint, because those recommenders are > > dynamic > > > > and > > > > > > > depend > > > > > > > > > > > on an actual configuration. > > > > > > > > > > > > > > > > > > > > > > 5) There are two relevant lines in the KIP: "If a > > .version > > > > > > property > > > > > > > > > > > contains a hard requirement, select the latest > installed > > > > > version > > > > > > > which > > > > > > > > > > > satisfies the requirement." and "This configuration is > > > > > > re-evaluated > > > > > > > > > > > each time the connector or task are assigned to a new > > > > worker". > > > > > I > > > > > > > would > > > > > > > > > > > call this "eager" upgrade behavior, rather than a > > "sticky" > > > or > > > > > > > "lazy" > > > > > > > > > > > upgrade behavior. > > > > > > > > > > > > > > > > > > > > > > 6) Updated! > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > Greg > > > > > > > > > > > > > > > > > > > > > > On Tue, May 14, 2024 at 9:14 AM Chris Egerton > > > > > > > <chr...@aiven.io.invalid > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > Thanks Greg for updating the KIP, and thanks > Snehashis > > > for > > > > > > > starting > > > > > > > > > the > > > > > > > > > > > > work on this originally. > > > > > > > > > > > > > > > > > > > > > > > > The motivation section makes a pretty convincing case > > for > > > > > this > > > > > > > kind > > > > > > > > > of > > > > > > > > > > > > feature. My thoughts are mostly about specific > details: > > > > > > > > > > > > > > > > > > > > > > > > 1) I like the support for version ranges (the example > > > > > > > demonstrating > > > > > > > > > how > > > > > > > > > > > to > > > > > > > > > > > > avoid KAFKA-10574 with the header converter was > > > > particularly > > > > > > > > > > > entertaining), > > > > > > > > > > > > but the configuration syntax for the most basic use > > case > > > of > > > > > > > > > specifying a > > > > > > > > > > > > single desired version is pretty counterintuitive. > > People > > > > may > > > > > > get > > > > > > > > > bitten > > > > > > > > > > > or > > > > > > > > > > > > at least frustrated if they put > connector.version=3.8.0 > > > in > > > > a > > > > > > > config > > > > > > > > > but > > > > > > > > > > > > then version 3.7.5 ends up running. I'd like it if we > > > could > > > > > > > either > > > > > > > > > > > > intentionally deviate from Maven ranges when a bare > > > version > > > > > is > > > > > > > > > present, > > > > > > > > > > > or > > > > > > > > > > > > separate things out into two properties: foo.version > > > would > > > > be > > > > > > the > > > > > > > > > single > > > > > > > > > > > > accepted version for the foo plugin, and > > > foo.version.range > > > > > > would > > > > > > > use > > > > > > > > > > > Maven > > > > > > > > > > > > range syntax. Open to other options too, just > > providing a > > > > > > couple > > > > > > > to > > > > > > > > > get > > > > > > > > > > > the > > > > > > > > > > > > ball rolling. > > > > > > > > > > > > > > > > > > > > > > > > 2) Although the current behavior for a worker with an > > > > invalid > > > > > > > > > > > > key/value/header converter class specified in its > > config > > > > file > > > > > > is > > > > > > > a > > > > > > > > > little > > > > > > > > > > > > strange (I was surprised to learn that it wouldn't > fail > > > on > > > > > > > startup), > > > > > > > > > I > > > > > > > > > > > > don't see a good reason to deviate from this when an > > > > invalid > > > > > > > version > > > > > > > > > is > > > > > > > > > > > > specified. Failing startup is drastic and has the > > > potential > > > > > to > > > > > > > > > disrupt > > > > > > > > > > > the > > > > > > > > > > > > availability of connectors that would otherwise be > able > > > to > > > > > run > > > > > > > > > healthily > > > > > > > > > > > > because they were explicitly configured to use valid > > > > > converters > > > > > > > > > instead > > > > > > > > > > > of > > > > > > > > > > > > the worker defaults. > > > > > > > > > > > > > > > > > > > > > > > > 3) Why are metrics utilized to report information > about > > > > > plugin > > > > > > > > > versions > > > > > > > > > > > > utilized by connectors at runtime instead of > publishing > > > > this > > > > > > > info in > > > > > > > > > the > > > > > > > > > > > > REST API? I saw that this was mentioned as a rejected > > > > > > > alternative, > > > > > > > > > but I > > > > > > > > > > > > didn't get a sense of why. It seems like the REST API > > > would > > > > > be > > > > > > > > > easier to > > > > > > > > > > > > access and more intuitive for most users than new > > > metrics. > > > > > > > > > > > > > > > > > > > > > > > > 4) In the "Validation" section it's stated that > "Users > > > can > > > > > use > > > > > > > these > > > > > > > > > > > > recommenders to discover the valid plugin classes and > > > > > versions, > > > > > > > > > without > > > > > > > > > > > > requiring an earlier call to GET > > > > > > > > > > > /connector-plugins?connectorsOnly=false." > > > > > > > > > > > > I really like the creativity and simplicity of > reusing > > > the > > > > > > > > > recommender > > > > > > > > > > > > mechanism to expose available versions for plugins > via > > > the > > > > > REST > > > > > > > API. > > > > > > > > > I'm > > > > > > > > > > > > unclear on whether or not it'll be possible to see > this > > > > > > > information > > > > > > > > > via > > > > > > > > > > > the > > > > > > > > > > > > GET /connector-plugins/<plugin>/config endpoint, > > though. > > > > It'd > > > > > > be > > > > > > > > > great if > > > > > > > > > > > > this were supported, since we learned in KIP-769 [1] > > that > > > > > > people > > > > > > > > > really > > > > > > > > > > > > want to be able to see configuration options for > > > connectors > > > > > and > > > > > > > their > > > > > > > > > > > > plugins via some kind of GET endpoint without having > to > > > > > > provide a > > > > > > > > > > > complete > > > > > > > > > > > > connector config for validation. > > > > > > > > > > > > > > > > > > > > > > > > 5) In the Maven version range docs, it's stated that > > > "Maven > > > > > > > picks the > > > > > > > > > > > > highest version of each project that satisfies all > the > > > hard > > > > > > > > > requirements > > > > > > > > > > > of > > > > > > > > > > > > the dependencies on that project." I'm guessing this > > > > behavior > > > > > > > will be > > > > > > > > > > > > retained for Connect; i.e., the highest-possible > > version > > > of > > > > > > each > > > > > > > > > plugin > > > > > > > > > > > > that satisfies the user-specified version constraints > > > will > > > > be > > > > > > > run? > > > > > > > > > (An > > > > > > > > > > > > alternative approach could be to have some kind of > > > "sticky" > > > > > > logic > > > > > > > > > that > > > > > > > > > > > only > > > > > > > > > > > > restarts connectors/tasks when their currently-used > > > version > > > > > > > becomes > > > > > > > > > > > > incompatible with the configured constraints.) > > > > > > > > > > > > > > > > > > > > > > > > 6) (Nit) It'd be nice to add a link to the > TestPlugins > > > > class > > > > > or > > > > > > > > > somewhere > > > > > > > > > > > > in its neighborhood to the testing plan; unfamiliar > > > readers > > > > > > > probably > > > > > > > > > > > won't > > > > > > > > > > > > get much out of what's there right now. > > > > > > > > > > > > > > > > > > > > > > > > [1] - > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-769%3A+Connect+APIs+to+list+all+connector+plugins+and+retrieve+their+configuration+definitions > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > On Mon, May 13, 2024 at 2:01 PM Snehashis < > > > > > > > snehashisp1...@gmail.com> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Hi Greg, > > > > > > > > > > > > > > > > > > > > > > > > > > That is much appreciated. No complaints on the > > > additional > > > > > > > scope, I > > > > > > > > > will > > > > > > > > > > > > > make some time out to work on this once we have > > > approval. > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > Snehashis > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, May 10, 2024 at 9:28 PM Greg Harris > > > > > > > > > > > <greg.har...@aiven.io.invalid> > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Snehashis, > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm glad to hear you're still interested in this > > KIP! > > > > > > > > > > > > > > I'm happy to let you drive this, and I apologize > > for > > > > > > > increasing > > > > > > > > > the > > > > > > > > > > > > > > scope of work so drastically. To make up for > that, > > > I'll > > > > > > > > > volunteer to > > > > > > > > > > > > > > be the primary PR reviewer to help get this done > > > > quickly > > > > > > once > > > > > > > > > the KIP > > > > > > > > > > > > > > is approved. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Greg > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, May 10, 2024 at 3:51 AM Snehashis < > > > > > > > > > snehashisp1...@gmail.com> > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Greg, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the follow up to my original KIP, I > am > > > in > > > > > > > favour of > > > > > > > > > the > > > > > > > > > > > > > > > additions made to expand its scope, the > addition > > of > > > > > range > > > > > > > > > versions > > > > > > > > > > > > > > > specifically make a lot of sense. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Apologies if I have not publicly worked on this > > KIP > > > > > for a > > > > > > > long > > > > > > > > > > > time. > > > > > > > > > > > > > The > > > > > > > > > > > > > > > original work was done when the move to service > > > > loading > > > > > > > was in > > > > > > > > > > > > > discussion > > > > > > > > > > > > > > > and I wanted to loop back to this only after > that > > > > work > > > > > > was > > > > > > > > > > > completed. > > > > > > > > > > > > > > Post > > > > > > > > > > > > > > > its conclusion, I have not been able to take > this > > > up > > > > > due > > > > > > to > > > > > > > > > other > > > > > > > > > > > > > > > priorities. If it's okay with you, I would > still > > > like > > > > > to > > > > > > > get > > > > > > > > > this > > > > > > > > > > > > > > > implemented myself, including the additional > > scope. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks and regards > > > > > > > > > > > > > > > Snehashis > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, May 10, 2024 at 12:45 AM Greg Harris > > > > > > > > > > > > > > <greg.har...@aiven.io.invalid> > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'd like to reboot the discussion on KIP-891: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-891%3A+Running+multiple+versions+of+Connector+plugins > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I've made some changes, most notably: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Specifying versions for all plugins in > > > Connector > > > > > > > configs > > > > > > > > > > > > > > > > (converters, header converters, transforms, > and > > > > > > > predicates) > > > > > > > > > not > > > > > > > > > > > just > > > > > > > > > > > > > > > > connectors & tasks > > > > > > > > > > > > > > > > 2. Specifying a range of versions instead of > an > > > > exact > > > > > > > match > > > > > > > > > > > > > > > > 3. New metrics to observe what versions are > > > in-use > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks to Snehashis for the original KIP > idea! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > Greg > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Jan 2, 2024 at 11:49 AM Greg Harris < > > > > > > > > > > > greg.har...@aiven.io> > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Snehashis, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you for the KIP! This is something > I've > > > > > wanted > > > > > > > for a > > > > > > > > > long > > > > > > > > > > > > > time. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I know the discussion has gone cold, are > you > > > > still > > > > > > > > > interested > > > > > > > > > > > in > > > > > > > > > > > > > > > > > pursuing this feature? I'll make time to > > review > > > > the > > > > > > > KIP if > > > > > > > > > you > > > > > > > > > > > are > > > > > > > > > > > > > > > > > still accepting comments. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > Greg > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 12:29 PM Snehashis > < > > > > > > > > > > > > > snehashisp1...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the points Sagar. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1) Should we update the GET /connectors > > > > > endpoint > > > > > > to > > > > > > > > > > > include the > > > > > > > > > > > > > > > > version of > > > > > > > > > > > > > > > > > > > the plugin that is running? It could be > > > > useful > > > > > to > > > > > > > > > figure > > > > > > > > > > > out > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > version > > > > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > the plugin or I am assuming it gets > > > returned > > > > by > > > > > > the > > > > > > > > > > > expand=info > > > > > > > > > > > > > > call? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think this is good to have and possible > > > > future > > > > > > > > > > > enhancement. The > > > > > > > > > > > > > > > > version > > > > > > > > > > > > > > > > > > info will be present in the config of the > > > > > connector > > > > > > > if > > > > > > > > > the > > > > > > > > > > > user > > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > > specified the version. Otherwise it is > the > > > > latest > > > > > > > version > > > > > > > > > > > which > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > user > > > > > > > > > > > > > > > > > > can find out from the connector-plugin > > > > endpoint. > > > > > > The > > > > > > > > > > > information > > > > > > > > > > > > > > can be > > > > > > > > > > > > > > > > > > introduced to the response of the GET > > > > /connectors > > > > > > > > > endpoint > > > > > > > > > > > > > itself, > > > > > > > > > > > > > > > > however > > > > > > > > > > > > > > > > > > the most ideal way of doing this would be > > to > > > > get > > > > > > the > > > > > > > > > > > currently > > > > > > > > > > > > > > running > > > > > > > > > > > > > > > > > > instance of the connector and get the > > version > > > > > > > directly > > > > > > > > > from > > > > > > > > > > > > > there. > > > > > > > > > > > > > > > > This is > > > > > > > > > > > > > > > > > > slightly tricky as the connector could be > > > > running > > > > > > in > > > > > > > a > > > > > > > > > > > different > > > > > > > > > > > > > > node. > > > > > > > > > > > > > > > > > > One way to do this would be to persist > the > > > > > version > > > > > > > > > > > information in > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > status backing store during instantiation > > of > > > > the > > > > > > > > > connector. > > > > > > > > > > > It > > > > > > > > > > > > > > requires > > > > > > > > > > > > > > > > > > some more thought and since the version > is > > > part > > > > > of > > > > > > > the > > > > > > > > > > > configs if > > > > > > > > > > > > > > > > provided > > > > > > > > > > > > > > > > > > and evident otherwise, I have not > included > > it > > > > in > > > > > > this > > > > > > > > > KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2) I am not aware of this and hence > > asking, > > > > > can 2 > > > > > > > > > > > connectors > > > > > > > > > > > > > with > > > > > > > > > > > > > > > > > > different > > > > > > > > > > > > > > > > > > > versions have the same name? Does the > > > plugin > > > > > > > isolation > > > > > > > > > > > allow > > > > > > > > > > > > > > this? > > > > > > > > > > > > > > > > This > > > > > > > > > > > > > > > > > > > could have a bearing when using the > > > lifecycle > > > > > > > > > endpoints for > > > > > > > > > > > > > > > > connectors > > > > > > > > > > > > > > > > > > like > > > > > > > > > > > > > > > > > > > DELETE etc. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > All connectors in a cluster need to have > > > > uniquire > > > > > > > > > connector > > > > > > > > > > > names > > > > > > > > > > > > > > > > > > regardless of what version of the plugin > > the > > > > > > > connector is > > > > > > > > > > > running > > > > > > > > > > > > > > > > > > underneath. This is something enforced by > > the > > > > > > connect > > > > > > > > > runtime > > > > > > > > > > > > > > itself. > > > > > > > > > > > > > > > > All > > > > > > > > > > > > > > > > > > connect CRUD operations are keyed on the > > > > > connector > > > > > > > name > > > > > > > > > so > > > > > > > > > > > there > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > > be an issue. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regards > > > > > > > > > > > > > > > > > > Snehashis > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 3:16 PM Sagar < > > > > > > > > > > > sagarmeansoc...@gmail.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Snehashsih, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP. It looks like a > very > > > > useful > > > > > > > > > feature. > > > > > > > > > > > Couple > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > > > small-ish points, let me know what you > > > think: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1) Should we update the GET /connectors > > > > > endpoint > > > > > > to > > > > > > > > > > > include the > > > > > > > > > > > > > > > > version of > > > > > > > > > > > > > > > > > > > the plugin that is running? It could be > > > > useful > > > > > to > > > > > > > > > figure > > > > > > > > > > > out > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > version of > > > > > > > > > > > > > > > > > > > the plugin or I am assuming it gets > > > returned > > > > by > > > > > > the > > > > > > > > > > > expand=info > > > > > > > > > > > > > > call? > > > > > > > > > > > > > > > > > > > 2) I am not aware of this and hence > > asking, > > > > > can 2 > > > > > > > > > > > connectors > > > > > > > > > > > > > with > > > > > > > > > > > > > > > > different > > > > > > > > > > > > > > > > > > > versions have the same name? Does the > > > plugin > > > > > > > isolation > > > > > > > > > > > allow > > > > > > > > > > > > > > this? > > > > > > > > > > > > > > > > This > > > > > > > > > > > > > > > > > > > could have a bearing when using the > > > lifecycle > > > > > > > > > endpoints for > > > > > > > > > > > > > > > > connectors like > > > > > > > > > > > > > > > > > > > DELETE etc. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > > Sagar. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 2:10 PM Ashwin > > > > > > > > > > > > > > <apan...@confluent.io.invalid > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Snehasis, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > IIUC (please correct me if I am > wrong > > > > > here), > > > > > > > what > > > > > > > > > you > > > > > > > > > > > > > > highlighted > > > > > > > > > > > > > > > > > > > above, > > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > a versioning scheme for a connector > > > config > > > > > for > > > > > > > the > > > > > > > > > same > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > (and > > > > > > > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > > > > different versions of a connector > > > plugin). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sorry for not being more precise in > my > > > > > wording > > > > > > > - I > > > > > > > > > meant > > > > > > > > > > > > > > > > registering > > > > > > > > > > > > > > > > > > > > versions of schema for connector > > config. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Let's take the example of a fictional > > > > > connector > > > > > > > which > > > > > > > > > > > uses a > > > > > > > > > > > > > > > > fictional > > > > > > > > > > > > > > > > > > > AWS > > > > > > > > > > > > > > > > > > > > service. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Fictional Connector Config schema > > > > version:2.0 > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > { > > > > > > > > > > > > > > > > > > > > "$schema": " > > > > > > > > > http://json-schema.org/draft-04/schema#", > > > > > > > > > > > > > > > > > > > > "type": "object", > > > > > > > > > > > > > > > > > > > > "properties": { > > > > > > > > > > > > > > > > > > > > "name": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "schema_version": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "aws_access_key": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "aws_secret_key": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "required": [ > > > > > > > > > > > > > > > > > > > > "name", > > > > > > > > > > > > > > > > > > > > "schema_version", > > > > > > > > > > > > > > > > > > > > "aws_access_key", > > > > > > > > > > > > > > > > > > > > "aws_secret_key" > > > > > > > > > > > > > > > > > > > > ] > > > > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Fictional Connector config schema > > > > version:3.0 > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > { > > > > > > > > > > > > > > > > > > > > "$schema": " > > > > > > > > > http://json-schema.org/draft-04/schema#", > > > > > > > > > > > > > > > > > > > > "type": "object", > > > > > > > > > > > > > > > > > > > > "properties": { > > > > > > > > > > > > > > > > > > > > "name": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "schema_version": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "iam_role": { > > > > > > > > > > > > > > > > > > > > "type": "string" > > > > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > }, > > > > > > > > > > > > > > > > > > > > "required": [ > > > > > > > > > > > > > > > > > > > > "name", > > > > > > > > > > > > > > > > > > > > "schema_version", > > > > > > > > > > > > > > > > > > > > "iam_role" > > > > > > > > > > > > > > > > > > > > ] > > > > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The connector which supports > Fictional > > > > config > > > > > > > schema > > > > > > > > > 2.0 > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > validate > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > access key and secret key. > > > > > > > > > > > > > > > > > > > > Whereas a connector which supports > > config > > > > > with > > > > > > > schema > > > > > > > > > > > version > > > > > > > > > > > > > > 3.0 > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > > only > > > > > > > > > > > > > > > > > > > > validate the IAM role. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is the alternative which I > wanted > > to > > > > > > > suggest. > > > > > > > > > Each > > > > > > > > > > > > > plugin > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > > > register the schema versions of > > connector > > > > > > config > > > > > > > > > which it > > > > > > > > > > > > > > supports. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The plugin paths may be optionally > > > > different > > > > > > > i.e we > > > > > > > > > > > don't > > > > > > > > > > > > > > have to > > > > > > > > > > > > > > > > > > > > mandatorily add a new plugin path to > > > > support > > > > > a > > > > > > > new > > > > > > > > > schema > > > > > > > > > > > > > > version. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > Ashwin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 12:47 PM > > > Snehashis > > > > < > > > > > > > > > > > > > > > > snehashisp1...@gmail.com> > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the input Ashwin. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Can you elaborate on the > > rejected > > > > > > > > > alternatives ? > > > > > > > > > > > > > Suppose > > > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > > > > > > > config is versioned and has a > > schema. > > > > > Then > > > > > > a > > > > > > > > > single > > > > > > > > > > > > > plugin > > > > > > > > > > > > > > > > (whose > > > > > > > > > > > > > > > > > > > > > > dependencies have not changed) > can > > > > handle > > > > > > > > > multiple > > > > > > > > > > > config > > > > > > > > > > > > > > > > versions > > > > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > same connector class. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > IIUC (please correct me if I am > wrong > > > > > here), > > > > > > > what > > > > > > > > > you > > > > > > > > > > > > > > highlighted > > > > > > > > > > > > > > > > > > > above, > > > > > > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > > > > > a versioning scheme for a connector > > > > config > > > > > > for > > > > > > > the > > > > > > > > > same > > > > > > > > > > > > > > > > connector (and > > > > > > > > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > > > > > different versions of a connector > > > > plugin). > > > > > > > That is > > > > > > > > > a > > > > > > > > > > > > > somewhat > > > > > > > > > > > > > > > > > > > tangential > > > > > > > > > > > > > > > > > > > > > problem. While it is definitely a > > > useful > > > > > > > feature to > > > > > > > > > > > have, > > > > > > > > > > > > > > like a > > > > > > > > > > > > > > > > log to > > > > > > > > > > > > > > > > > > > > > check what changes were made over > > time > > > to > > > > > the > > > > > > > > > config > > > > > > > > > > > which > > > > > > > > > > > > > > might > > > > > > > > > > > > > > > > make > > > > > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > > > > easier to do rollbacks, it is not > the > > > > focus > > > > > > > here. > > > > > > > > > Here > > > > > > > > > > > by > > > > > > > > > > > > > > > > version we > > > > > > > > > > > > > > > > > > > mean > > > > > > > > > > > > > > > > > > > > > to say what underlying version of > the > > > > > plugin > > > > > > > > > should the > > > > > > > > > > > > > given > > > > > > > > > > > > > > > > > > > > configuration > > > > > > > > > > > > > > > > > > > > > of the connector use. Perhaps it is > > > > better > > > > > to > > > > > > > > > change > > > > > > > > > > > the > > > > > > > > > > > > > > name of > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > parameter from connector.version to > > > > > > > > > > > > > connector.plugin.version > > > > > > > > > > > > > > or > > > > > > > > > > > > > > > > > > > > > plugin.version if it was confusing. > > > wdyt? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Any plans to support assisted > > > > > migration > > > > > > > e.g > > > > > > > > > if a > > > > > > > > > > > user > > > > > > > > > > > > > > > > invokes > > > > > > > > > > > > > > > > > > > "POST > > > > > > > > > > > > > > > > > > > > > > connector/config?migrate=latest", > > the > > > > > > latest > > > > > > > > > version > > > > > > > > > > > > > > > > __attempts__ to > > > > > > > > > > > > > > > > > > > > > > transform the existing config to > > the > > > > > newer > > > > > > > > > version. > > > > > > > > > > > This > > > > > > > > > > > > > > would > > > > > > > > > > > > > > > > > > > require > > > > > > > > > > > > > > > > > > > > > > adding a method like "boolean > > > > > > migrate(Version > > > > > > > > > > > > > > fromVersion)" to > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > connector interface. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is an enhancement we can think > > of > > > > > doing > > > > > > in > > > > > > > > > future. > > > > > > > > > > > > > > Users can > > > > > > > > > > > > > > > > > > > simply > > > > > > > > > > > > > > > > > > > > do > > > > > > > > > > > > > > > > > > > > > a PUT call with the updated config > > > which > > > > > has > > > > > > > the > > > > > > > > > > > updated > > > > > > > > > > > > > > version > > > > > > > > > > > > > > > > > > > number. > > > > > > > > > > > > > > > > > > > > > The assisted mode could be handy as > > the > > > > > user > > > > > > > does > > > > > > > > > not > > > > > > > > > > > need > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > know the > > > > > > > > > > > > > > > > > > > > > config but beyond this it does not > > seem > > > > to > > > > > > > justify > > > > > > > > > its > > > > > > > > > > > > > > existence. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regards > > > > > > > > > > > > > > > > > > > > > Snehashis > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 22, 2022 at 10:50 AM > > Ashwin > > > > > > > > > > > > > > > > <apan...@confluent.io.invalid> > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Snehasis, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is a really useful feature > and > > > > > thanks > > > > > > > for > > > > > > > > > > > initiating > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > > > discussion. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I had the following questions - > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Can you elaborate on the > > rejected > > > > > > > > > alternatives ? > > > > > > > > > > > > > Suppose > > > > > > > > > > > > > > > > connector > > > > > > > > > > > > > > > > > > > > > > config is versioned and has a > > schema. > > > > > Then > > > > > > a > > > > > > > > > single > > > > > > > > > > > > > plugin > > > > > > > > > > > > > > > > (whose > > > > > > > > > > > > > > > > > > > > > > dependencies have not changed) > can > > > > handle > > > > > > > > > multiple > > > > > > > > > > > config > > > > > > > > > > > > > > > > versions > > > > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > same connector class. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Any plans to support assisted > > > > > migration > > > > > > > e.g > > > > > > > > > if a > > > > > > > > > > > user > > > > > > > > > > > > > > > > invokes > > > > > > > > > > > > > > > > > > > "POST > > > > > > > > > > > > > > > > > > > > > > connector/config?migrate=latest", > > the > > > > > > latest > > > > > > > > > version > > > > > > > > > > > > > > > > __attempts__ to > > > > > > > > > > > > > > > > > > > > > > transform the existing config to > > the > > > > > newer > > > > > > > > > version. > > > > > > > > > > > This > > > > > > > > > > > > > > would > > > > > > > > > > > > > > > > > > > require > > > > > > > > > > > > > > > > > > > > > > adding a method like "boolean > > > > > > migrate(Version > > > > > > > > > > > > > > fromVersion)" to > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > connector interface. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > Ashwin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 21, 2022 at 2:27 PM > > > > > Snehashis < > > > > > > > > > > > > > > > > snehashisp1...@gmail.com> > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'd like to start a discussion > > > thread > > > > > on > > > > > > > > > KIP-891: > > > > > > > > > > > > > Running > > > > > > > > > > > > > > > > multiple > > > > > > > > > > > > > > > > > > > > > > versions > > > > > > > > > > > > > > > > > > > > > > > of a connector. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The KIP aims to add the ability > > for > > > > the > > > > > > > connect > > > > > > > > > > > runtime > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > run > > > > > > > > > > > > > > > > > > > > multiple > > > > > > > > > > > > > > > > > > > > > > > versions of a connector. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-891%3A+Running+multiple+versions+of+a+connector > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please take a look and let me > > know > > > > what > > > > > > you > > > > > > > > > think. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you > > > > > > > > > > > > > > > > > > > > > > > Snehashis Pal > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >