Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
No further comments from me. On Thu, Mar 28, 2024, 19:36 Ivan Yurchenko wrote: > Hello Chris, > > Thanks for your feedback. I created the jira and also updated the > description a bit mentioning that other similar race scenarios exist and > the KIP is not trying to solve them. > > Best, > Ivan > > On Wed, Mar 27, 2024, at 17:08, Chris Egerton wrote: > > Hi Ivan, > > > > Thanks for the updates. LGTM! > > > > RE atomicity: I think it should be possible and not _too_ invasive to > > detect and handle these kinds of races by tracking the offset in the > config > > topic for connector configs and aborting an operation if that offset > > changes between when the request was initiated and when the write to the > > config topic will take place, but since the same kind of issue is also > > possible with other connector operations (concurrent configuration PUTs, > > for example) due to how validation is split out into a separate thread, I > > agree that it's not worth blocking the KIP on fixing this. > > > > One final nit: Can you update the Jira ticket link in the KIP? > > > > Cheers, > > > > Chris > > > > On Wed, Mar 27, 2024 at 2:56 PM Ivan Yurchenko wrote: > > > > > Hi, > > > > > > I updated the KIP with the two following changes: > > > 1. Using `null` values as tombstone value for removing existing fields > > > from configuration. > > > 2. Added a note about the lack of 100% atomicity, which seems very > > > difficult to achieve practically. > > > > > > Ivan > > > > > > > > > On Tue, Mar 26, 2024, at 14:45, Ivan Yurchenko wrote: > > > > Speaking of the Chris' comment > > > > > > > > > One thought that comes to mind is that sometimes it may be useful > to > > > > > explicitly remove properties from a connector configuration. We > might > > > > > permit this by allowing users to specify null (the JSON literal, > not a > > > > > string containing the characters "null") as the value for > to-be-removed > > > > > properties. > > > > > > > > This actually makes sense. AFAIU, `null` cannot be in the connector > > > config since https://github.com/apache/kafka/pull/11333, so using it > as a > > > tombstone value is a good idea. I can update the KIP. > > > > > > > > Ivan > > > > > > > > > > > > On Tue, Mar 26, 2024, at 14:19, Ivan Yurchenko wrote: > > > > > Hi all, > > > > > > > > > > This KIP is a bit old now :) but I think its context hasn't changed > > > much since then and the KIP is still valid. I would like to finally > bring > > > it to some conclusion. > > > > > > > > > > Best, > > > > > Ivan > > > > > > > > > > On 2021/07/12 14:49:47 Chris Egerton wrote: > > > > > > Hi all, > > > > > > > > > > > > Know it's been a while for this KIP but in my personal experience > > > the value > > > > > > of a PATCH method in the REST API has actually increased over > time > > > and I'd > > > > > > love to have this option for quick, stop-the-bleeding remediation > > > efforts. > > > > > > > > > > > > One thought that comes to mind is that sometimes it may be > useful to > > > > > > explicitly remove properties from a connector configuration. We > might > > > > > > permit this by allowing users to specify null (the JSON literal, > not > > > a > > > > > > string containing the characters "null") as the value for > > > to-be-removed > > > > > > properties. > > > > > > > > > > > > I'd love to see this change if you're still interested in driving > > > it, Ivan. > > > > > > Hopefully we can give it the attention it deserves in the > upcoming > > > months! > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Chris > > > > > > > > > > > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > > > > > > wrote: > > > > > > > > > > > > > Thank you for your feedback Ryanne! > > > > > > > These are all surely valid concerns and PATCH isn't really > > > necessary or > > > > > > > suitable for normal production configuration management. > However, > > > there are > > > > > > > cases where quick patching of the configuration is useful, > such as > > > hot > > > > > > > fixes of production or in development. > > > > > > > > > > > > > > Overall, the change itself is really tiny and if the > cost-benefit > > > balance > > > > > > > is positive, I'd still like to drive it further. > > > > > > > > > > > > > > Ivan > > > > > > > > > > > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan > > > wrote: > > > > > > > > > > > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided > > > not to > > > > > > > pursue > > > > > > > > the idea for a few reasons: > > > > > > > > > > > > > > > > 1) PATCH is still racy. For example, if you want to add a > topic > > > to the > > > > > > > > "topics" property, you still need to read, modify, and write > the > > > existing > > > > > > > > value. To handle this, you'd need to support atomic > sub-document > > > > > > > > operations, which I don't see happening. > > > > > > > > > > > > > > > > 2) A common pattern is to store your configurations in git or > > > something, > > > > > > > > and then
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hello Chris, Thanks for your feedback. I created the jira and also updated the description a bit mentioning that other similar race scenarios exist and the KIP is not trying to solve them. Best, Ivan On Wed, Mar 27, 2024, at 17:08, Chris Egerton wrote: > Hi Ivan, > > Thanks for the updates. LGTM! > > RE atomicity: I think it should be possible and not _too_ invasive to > detect and handle these kinds of races by tracking the offset in the config > topic for connector configs and aborting an operation if that offset > changes between when the request was initiated and when the write to the > config topic will take place, but since the same kind of issue is also > possible with other connector operations (concurrent configuration PUTs, > for example) due to how validation is split out into a separate thread, I > agree that it's not worth blocking the KIP on fixing this. > > One final nit: Can you update the Jira ticket link in the KIP? > > Cheers, > > Chris > > On Wed, Mar 27, 2024 at 2:56 PM Ivan Yurchenko wrote: > > > Hi, > > > > I updated the KIP with the two following changes: > > 1. Using `null` values as tombstone value for removing existing fields > > from configuration. > > 2. Added a note about the lack of 100% atomicity, which seems very > > difficult to achieve practically. > > > > Ivan > > > > > > On Tue, Mar 26, 2024, at 14:45, Ivan Yurchenko wrote: > > > Speaking of the Chris' comment > > > > > > > One thought that comes to mind is that sometimes it may be useful to > > > > explicitly remove properties from a connector configuration. We might > > > > permit this by allowing users to specify null (the JSON literal, not a > > > > string containing the characters "null") as the value for to-be-removed > > > > properties. > > > > > > This actually makes sense. AFAIU, `null` cannot be in the connector > > config since https://github.com/apache/kafka/pull/11333, so using it as a > > tombstone value is a good idea. I can update the KIP. > > > > > > Ivan > > > > > > > > > On Tue, Mar 26, 2024, at 14:19, Ivan Yurchenko wrote: > > > > Hi all, > > > > > > > > This KIP is a bit old now :) but I think its context hasn't changed > > much since then and the KIP is still valid. I would like to finally bring > > it to some conclusion. > > > > > > > > Best, > > > > Ivan > > > > > > > > On 2021/07/12 14:49:47 Chris Egerton wrote: > > > > > Hi all, > > > > > > > > > > Know it's been a while for this KIP but in my personal experience > > the value > > > > > of a PATCH method in the REST API has actually increased over time > > and I'd > > > > > love to have this option for quick, stop-the-bleeding remediation > > efforts. > > > > > > > > > > One thought that comes to mind is that sometimes it may be useful to > > > > > explicitly remove properties from a connector configuration. We might > > > > > permit this by allowing users to specify null (the JSON literal, not > > a > > > > > string containing the characters "null") as the value for > > to-be-removed > > > > > properties. > > > > > > > > > > I'd love to see this change if you're still interested in driving > > it, Ivan. > > > > > Hopefully we can give it the attention it deserves in the upcoming > > months! > > > > > > > > > > Cheers, > > > > > > > > > > Chris > > > > > > > > > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > > > > > wrote: > > > > > > > > > > > Thank you for your feedback Ryanne! > > > > > > These are all surely valid concerns and PATCH isn't really > > necessary or > > > > > > suitable for normal production configuration management. However, > > there are > > > > > > cases where quick patching of the configuration is useful, such as > > hot > > > > > > fixes of production or in development. > > > > > > > > > > > > Overall, the change itself is really tiny and if the cost-benefit > > balance > > > > > > is positive, I'd still like to drive it further. > > > > > > > > > > > > Ivan > > > > > > > > > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan > > wrote: > > > > > > > > > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided > > not to > > > > > > pursue > > > > > > > the idea for a few reasons: > > > > > > > > > > > > > > 1) PATCH is still racy. For example, if you want to add a topic > > to the > > > > > > > "topics" property, you still need to read, modify, and write the > > existing > > > > > > > value. To handle this, you'd need to support atomic sub-document > > > > > > > operations, which I don't see happening. > > > > > > > > > > > > > > 2) A common pattern is to store your configurations in git or > > something, > > > > > > > and then apply them via PUT. Throw in some triggers or jenkins > > etc, and > > > > > > you > > > > > > > have a more robust solution than PATCH provides. > > > > > > > > > > > > > > 3) For properties that change a lot, it's possible to use an > > out-of-band > > > > > > > data source, e.g. Kafka or Zookeeper, and then have your > > Connector > > > > > > > subscribe to
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hi Ivan, Thanks for the updates. LGTM! RE atomicity: I think it should be possible and not _too_ invasive to detect and handle these kinds of races by tracking the offset in the config topic for connector configs and aborting an operation if that offset changes between when the request was initiated and when the write to the config topic will take place, but since the same kind of issue is also possible with other connector operations (concurrent configuration PUTs, for example) due to how validation is split out into a separate thread, I agree that it's not worth blocking the KIP on fixing this. One final nit: Can you update the Jira ticket link in the KIP? Cheers, Chris On Wed, Mar 27, 2024 at 2:56 PM Ivan Yurchenko wrote: > Hi, > > I updated the KIP with the two following changes: > 1. Using `null` values as tombstone value for removing existing fields > from configuration. > 2. Added a note about the lack of 100% atomicity, which seems very > difficult to achieve practically. > > Ivan > > > On Tue, Mar 26, 2024, at 14:45, Ivan Yurchenko wrote: > > Speaking of the Chris' comment > > > > > One thought that comes to mind is that sometimes it may be useful to > > > explicitly remove properties from a connector configuration. We might > > > permit this by allowing users to specify null (the JSON literal, not a > > > string containing the characters "null") as the value for to-be-removed > > > properties. > > > > This actually makes sense. AFAIU, `null` cannot be in the connector > config since https://github.com/apache/kafka/pull/11333, so using it as a > tombstone value is a good idea. I can update the KIP. > > > > Ivan > > > > > > On Tue, Mar 26, 2024, at 14:19, Ivan Yurchenko wrote: > > > Hi all, > > > > > > This KIP is a bit old now :) but I think its context hasn't changed > much since then and the KIP is still valid. I would like to finally bring > it to some conclusion. > > > > > > Best, > > > Ivan > > > > > > On 2021/07/12 14:49:47 Chris Egerton wrote: > > > > Hi all, > > > > > > > > Know it's been a while for this KIP but in my personal experience > the value > > > > of a PATCH method in the REST API has actually increased over time > and I'd > > > > love to have this option for quick, stop-the-bleeding remediation > efforts. > > > > > > > > One thought that comes to mind is that sometimes it may be useful to > > > > explicitly remove properties from a connector configuration. We might > > > > permit this by allowing users to specify null (the JSON literal, not > a > > > > string containing the characters "null") as the value for > to-be-removed > > > > properties. > > > > > > > > I'd love to see this change if you're still interested in driving > it, Ivan. > > > > Hopefully we can give it the attention it deserves in the upcoming > months! > > > > > > > > Cheers, > > > > > > > > Chris > > > > > > > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > > > > wrote: > > > > > > > > > Thank you for your feedback Ryanne! > > > > > These are all surely valid concerns and PATCH isn't really > necessary or > > > > > suitable for normal production configuration management. However, > there are > > > > > cases where quick patching of the configuration is useful, such as > hot > > > > > fixes of production or in development. > > > > > > > > > > Overall, the change itself is really tiny and if the cost-benefit > balance > > > > > is positive, I'd still like to drive it further. > > > > > > > > > > Ivan > > > > > > > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan > wrote: > > > > > > > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided > not to > > > > > pursue > > > > > > the idea for a few reasons: > > > > > > > > > > > > 1) PATCH is still racy. For example, if you want to add a topic > to the > > > > > > "topics" property, you still need to read, modify, and write the > existing > > > > > > value. To handle this, you'd need to support atomic sub-document > > > > > > operations, which I don't see happening. > > > > > > > > > > > > 2) A common pattern is to store your configurations in git or > something, > > > > > > and then apply them via PUT. Throw in some triggers or jenkins > etc, and > > > > > you > > > > > > have a more robust solution than PATCH provides. > > > > > > > > > > > > 3) For properties that change a lot, it's possible to use an > out-of-band > > > > > > data source, e.g. Kafka or Zookeeper, and then have your > Connector > > > > > > subscribe to changes. I've done something like this to enable > dynamic > > > > > > reconfiguration of Connectors from command-line tools and > dashboards > > > > > > without involving the Connect REST API at all. Moreover, I've > done so in > > > > > an > > > > > > atomic, non-racy way. > > > > > > > > > > > > So I don't think PATCH is strictly necessary nor sufficient for > atomic > > > > > > partial updates. That said, it doesn't hurt and I'm happy to > support the > > > > > > KIP. > > > > > > > > > > > > Ryanne > > > > > > > > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hi, I updated the KIP with the two following changes: 1. Using `null` values as tombstone value for removing existing fields from configuration. 2. Added a note about the lack of 100% atomicity, which seems very difficult to achieve practically. Ivan On Tue, Mar 26, 2024, at 14:45, Ivan Yurchenko wrote: > Speaking of the Chris' comment > > > One thought that comes to mind is that sometimes it may be useful to > > explicitly remove properties from a connector configuration. We might > > permit this by allowing users to specify null (the JSON literal, not a > > string containing the characters "null") as the value for to-be-removed > > properties. > > This actually makes sense. AFAIU, `null` cannot be in the connector config > since https://github.com/apache/kafka/pull/11333, so using it as a tombstone > value is a good idea. I can update the KIP. > > Ivan > > > On Tue, Mar 26, 2024, at 14:19, Ivan Yurchenko wrote: > > Hi all, > > > > This KIP is a bit old now :) but I think its context hasn't changed much > > since then and the KIP is still valid. I would like to finally bring it to > > some conclusion. > > > > Best, > > Ivan > > > > On 2021/07/12 14:49:47 Chris Egerton wrote: > > > Hi all, > > > > > > Know it's been a while for this KIP but in my personal experience the > > > value > > > of a PATCH method in the REST API has actually increased over time and I'd > > > love to have this option for quick, stop-the-bleeding remediation efforts. > > > > > > One thought that comes to mind is that sometimes it may be useful to > > > explicitly remove properties from a connector configuration. We might > > > permit this by allowing users to specify null (the JSON literal, not a > > > string containing the characters "null") as the value for to-be-removed > > > properties. > > > > > > I'd love to see this change if you're still interested in driving it, > > > Ivan. > > > Hopefully we can give it the attention it deserves in the upcoming months! > > > > > > Cheers, > > > > > > Chris > > > > > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > > > wrote: > > > > > > > Thank you for your feedback Ryanne! > > > > These are all surely valid concerns and PATCH isn't really necessary or > > > > suitable for normal production configuration management. However, there > > > > are > > > > cases where quick patching of the configuration is useful, such as hot > > > > fixes of production or in development. > > > > > > > > Overall, the change itself is really tiny and if the cost-benefit > > > > balance > > > > is positive, I'd still like to drive it further. > > > > > > > > Ivan > > > > > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan wrote: > > > > > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided not to > > > > pursue > > > > > the idea for a few reasons: > > > > > > > > > > 1) PATCH is still racy. For example, if you want to add a topic to the > > > > > "topics" property, you still need to read, modify, and write the > > > > > existing > > > > > value. To handle this, you'd need to support atomic sub-document > > > > > operations, which I don't see happening. > > > > > > > > > > 2) A common pattern is to store your configurations in git or > > > > > something, > > > > > and then apply them via PUT. Throw in some triggers or jenkins etc, > > > > > and > > > > you > > > > > have a more robust solution than PATCH provides. > > > > > > > > > > 3) For properties that change a lot, it's possible to use an > > > > > out-of-band > > > > > data source, e.g. Kafka or Zookeeper, and then have your Connector > > > > > subscribe to changes. I've done something like this to enable dynamic > > > > > reconfiguration of Connectors from command-line tools and dashboards > > > > > without involving the Connect REST API at all. Moreover, I've done so > > > > > in > > > > an > > > > > atomic, non-racy way. > > > > > > > > > > So I don't think PATCH is strictly necessary nor sufficient for atomic > > > > > partial updates. That said, it doesn't hurt and I'm happy to support > > > > > the > > > > > KIP. > > > > > > > > > > Ryanne > > > > > > > > > > On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko < > > > > ivan0yurche...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > Since Kafka 2.3 has just been release and more people may have time > > > > > > to > > > > > look > > > > > > at this now, I'd like to bump this discussion. > > > > > > Thanks. > > > > > > > > > > > > Ivan > > > > > > > > > > > > > > > > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > > > > > > connector config in Connect REST API. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Speaking of the Chris' comment > One thought that comes to mind is that sometimes it may be useful to > explicitly remove properties from a connector configuration. We might > permit this by allowing users to specify null (the JSON literal, not a > string containing the characters "null") as the value for to-be-removed > properties. This actually makes sense. AFAIU, `null` cannot be in the connector config since https://github.com/apache/kafka/pull/11333, so using it as a tombstone value is a good idea. I can update the KIP. Ivan On Tue, Mar 26, 2024, at 14:19, Ivan Yurchenko wrote: > Hi all, > > This KIP is a bit old now :) but I think its context hasn't changed much > since then and the KIP is still valid. I would like to finally bring it to > some conclusion. > > Best, > Ivan > > On 2021/07/12 14:49:47 Chris Egerton wrote: > > Hi all, > > > > Know it's been a while for this KIP but in my personal experience the value > > of a PATCH method in the REST API has actually increased over time and I'd > > love to have this option for quick, stop-the-bleeding remediation efforts. > > > > One thought that comes to mind is that sometimes it may be useful to > > explicitly remove properties from a connector configuration. We might > > permit this by allowing users to specify null (the JSON literal, not a > > string containing the characters "null") as the value for to-be-removed > > properties. > > > > I'd love to see this change if you're still interested in driving it, Ivan. > > Hopefully we can give it the attention it deserves in the upcoming months! > > > > Cheers, > > > > Chris > > > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > > wrote: > > > > > Thank you for your feedback Ryanne! > > > These are all surely valid concerns and PATCH isn't really necessary or > > > suitable for normal production configuration management. However, there > > > are > > > cases where quick patching of the configuration is useful, such as hot > > > fixes of production or in development. > > > > > > Overall, the change itself is really tiny and if the cost-benefit balance > > > is positive, I'd still like to drive it further. > > > > > > Ivan > > > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan wrote: > > > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided not to > > > pursue > > > > the idea for a few reasons: > > > > > > > > 1) PATCH is still racy. For example, if you want to add a topic to the > > > > "topics" property, you still need to read, modify, and write the > > > > existing > > > > value. To handle this, you'd need to support atomic sub-document > > > > operations, which I don't see happening. > > > > > > > > 2) A common pattern is to store your configurations in git or something, > > > > and then apply them via PUT. Throw in some triggers or jenkins etc, and > > > you > > > > have a more robust solution than PATCH provides. > > > > > > > > 3) For properties that change a lot, it's possible to use an out-of-band > > > > data source, e.g. Kafka or Zookeeper, and then have your Connector > > > > subscribe to changes. I've done something like this to enable dynamic > > > > reconfiguration of Connectors from command-line tools and dashboards > > > > without involving the Connect REST API at all. Moreover, I've done so in > > > an > > > > atomic, non-racy way. > > > > > > > > So I don't think PATCH is strictly necessary nor sufficient for atomic > > > > partial updates. That said, it doesn't hurt and I'm happy to support the > > > > KIP. > > > > > > > > Ryanne > > > > > > > > On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko < > > > ivan0yurche...@gmail.com> > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > Since Kafka 2.3 has just been release and more people may have time to > > > > look > > > > > at this now, I'd like to bump this discussion. > > > > > Thanks. > > > > > > > > > > Ivan > > > > > > > > > > > > > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > > > > > > > > wrote: > > > > > > > > > > > Hello, > > > > > > > > > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > > > > > connector config in Connect REST API. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > > > > > > > > > > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > > > > > > > > > > > Thank you. > > > > > > > > > > > > Ivan > > > > > > > > > > > > > > > > > > > >
RE: Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hi all, This KIP is a bit old now :) but I think its context hasn't changed much since then and the KIP is still valid. I would like to finally bring it to some conclusion. Best, Ivan On 2021/07/12 14:49:47 Chris Egerton wrote: > Hi all, > > Know it's been a while for this KIP but in my personal experience the value > of a PATCH method in the REST API has actually increased over time and I'd > love to have this option for quick, stop-the-bleeding remediation efforts. > > One thought that comes to mind is that sometimes it may be useful to > explicitly remove properties from a connector configuration. We might > permit this by allowing users to specify null (the JSON literal, not a > string containing the characters "null") as the value for to-be-removed > properties. > > I'd love to see this change if you're still interested in driving it, Ivan. > Hopefully we can give it the attention it deserves in the upcoming months! > > Cheers, > > Chris > > On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko > wrote: > > > Thank you for your feedback Ryanne! > > These are all surely valid concerns and PATCH isn't really necessary or > > suitable for normal production configuration management. However, there are > > cases where quick patching of the configuration is useful, such as hot > > fixes of production or in development. > > > > Overall, the change itself is really tiny and if the cost-benefit balance > > is positive, I'd still like to drive it further. > > > > Ivan > > > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan wrote: > > > > > Ivan, I looked at adding PATCH a while ago as well. I decided not to > > pursue > > > the idea for a few reasons: > > > > > > 1) PATCH is still racy. For example, if you want to add a topic to the > > > "topics" property, you still need to read, modify, and write the existing > > > value. To handle this, you'd need to support atomic sub-document > > > operations, which I don't see happening. > > > > > > 2) A common pattern is to store your configurations in git or something, > > > and then apply them via PUT. Throw in some triggers or jenkins etc, and > > you > > > have a more robust solution than PATCH provides. > > > > > > 3) For properties that change a lot, it's possible to use an out-of-band > > > data source, e.g. Kafka or Zookeeper, and then have your Connector > > > subscribe to changes. I've done something like this to enable dynamic > > > reconfiguration of Connectors from command-line tools and dashboards > > > without involving the Connect REST API at all. Moreover, I've done so in > > an > > > atomic, non-racy way. > > > > > > So I don't think PATCH is strictly necessary nor sufficient for atomic > > > partial updates. That said, it doesn't hurt and I'm happy to support the > > > KIP. > > > > > > Ryanne > > > > > > On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko < > > ivan0yurche...@gmail.com> > > > wrote: > > > > > > > Hi, > > > > > > > > Since Kafka 2.3 has just been release and more people may have time to > > > look > > > > at this now, I'd like to bump this discussion. > > > > Thanks. > > > > > > > > Ivan > > > > > > > > > > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > > > > > > wrote: > > > > > > > > > Hello, > > > > > > > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > > > > connector config in Connect REST API. > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > > > > > > > > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > > > > > > > > > Thank you. > > > > > > > > > > Ivan > > > > > > > > > > > > > > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hi all, Know it's been a while for this KIP but in my personal experience the value of a PATCH method in the REST API has actually increased over time and I'd love to have this option for quick, stop-the-bleeding remediation efforts. One thought that comes to mind is that sometimes it may be useful to explicitly remove properties from a connector configuration. We might permit this by allowing users to specify null (the JSON literal, not a string containing the characters "null") as the value for to-be-removed properties. I'd love to see this change if you're still interested in driving it, Ivan. Hopefully we can give it the attention it deserves in the upcoming months! Cheers, Chris On Fri, Jun 28, 2019 at 4:56 AM Ivan Yurchenko wrote: > Thank you for your feedback Ryanne! > These are all surely valid concerns and PATCH isn't really necessary or > suitable for normal production configuration management. However, there are > cases where quick patching of the configuration is useful, such as hot > fixes of production or in development. > > Overall, the change itself is really tiny and if the cost-benefit balance > is positive, I'd still like to drive it further. > > Ivan > > On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan wrote: > > > Ivan, I looked at adding PATCH a while ago as well. I decided not to > pursue > > the idea for a few reasons: > > > > 1) PATCH is still racy. For example, if you want to add a topic to the > > "topics" property, you still need to read, modify, and write the existing > > value. To handle this, you'd need to support atomic sub-document > > operations, which I don't see happening. > > > > 2) A common pattern is to store your configurations in git or something, > > and then apply them via PUT. Throw in some triggers or jenkins etc, and > you > > have a more robust solution than PATCH provides. > > > > 3) For properties that change a lot, it's possible to use an out-of-band > > data source, e.g. Kafka or Zookeeper, and then have your Connector > > subscribe to changes. I've done something like this to enable dynamic > > reconfiguration of Connectors from command-line tools and dashboards > > without involving the Connect REST API at all. Moreover, I've done so in > an > > atomic, non-racy way. > > > > So I don't think PATCH is strictly necessary nor sufficient for atomic > > partial updates. That said, it doesn't hurt and I'm happy to support the > > KIP. > > > > Ryanne > > > > On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko < > ivan0yurche...@gmail.com> > > wrote: > > > > > Hi, > > > > > > Since Kafka 2.3 has just been release and more people may have time to > > look > > > at this now, I'd like to bump this discussion. > > > Thanks. > > > > > > Ivan > > > > > > > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > > > > wrote: > > > > > > > Hello, > > > > > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > > > connector config in Connect REST API. > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > > > > > > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > > > > > > > Thank you. > > > > > > > > Ivan > > > > > > > > > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Thank you for your feedback Ryanne! These are all surely valid concerns and PATCH isn't really necessary or suitable for normal production configuration management. However, there are cases where quick patching of the configuration is useful, such as hot fixes of production or in development. Overall, the change itself is really tiny and if the cost-benefit balance is positive, I'd still like to drive it further. Ivan On Wed, 26 Jun 2019 at 17:45, Ryanne Dolan wrote: > Ivan, I looked at adding PATCH a while ago as well. I decided not to pursue > the idea for a few reasons: > > 1) PATCH is still racy. For example, if you want to add a topic to the > "topics" property, you still need to read, modify, and write the existing > value. To handle this, you'd need to support atomic sub-document > operations, which I don't see happening. > > 2) A common pattern is to store your configurations in git or something, > and then apply them via PUT. Throw in some triggers or jenkins etc, and you > have a more robust solution than PATCH provides. > > 3) For properties that change a lot, it's possible to use an out-of-band > data source, e.g. Kafka or Zookeeper, and then have your Connector > subscribe to changes. I've done something like this to enable dynamic > reconfiguration of Connectors from command-line tools and dashboards > without involving the Connect REST API at all. Moreover, I've done so in an > atomic, non-racy way. > > So I don't think PATCH is strictly necessary nor sufficient for atomic > partial updates. That said, it doesn't hurt and I'm happy to support the > KIP. > > Ryanne > > On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko > wrote: > > > Hi, > > > > Since Kafka 2.3 has just been release and more people may have time to > look > > at this now, I'd like to bump this discussion. > > Thanks. > > > > Ivan > > > > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > > wrote: > > > > > Hello, > > > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > > connector config in Connect REST API. > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > > > > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > > > > > Thank you. > > > > > > Ivan > > > > > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Ivan, I looked at adding PATCH a while ago as well. I decided not to pursue the idea for a few reasons: 1) PATCH is still racy. For example, if you want to add a topic to the "topics" property, you still need to read, modify, and write the existing value. To handle this, you'd need to support atomic sub-document operations, which I don't see happening. 2) A common pattern is to store your configurations in git or something, and then apply them via PUT. Throw in some triggers or jenkins etc, and you have a more robust solution than PATCH provides. 3) For properties that change a lot, it's possible to use an out-of-band data source, e.g. Kafka or Zookeeper, and then have your Connector subscribe to changes. I've done something like this to enable dynamic reconfiguration of Connectors from command-line tools and dashboards without involving the Connect REST API at all. Moreover, I've done so in an atomic, non-racy way. So I don't think PATCH is strictly necessary nor sufficient for atomic partial updates. That said, it doesn't hurt and I'm happy to support the KIP. Ryanne On Tue, Jun 25, 2019 at 12:15 PM Ivan Yurchenko wrote: > Hi, > > Since Kafka 2.3 has just been release and more people may have time to look > at this now, I'd like to bump this discussion. > Thanks. > > Ivan > > > On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko > wrote: > > > Hello, > > > > I'd like to start the discussion of KIP-477: Add PATCH method for > > connector config in Connect REST API. > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > > > Thank you. > > > > Ivan > > >
Re: [DISCUSS] KIP-477: Add PATCH method for connector config in Connect REST API
Hi, Since Kafka 2.3 has just been release and more people may have time to look at this now, I'd like to bump this discussion. Thanks. Ivan On Thu, 13 Jun 2019 at 17:20, Ivan Yurchenko wrote: > Hello, > > I'd like to start the discussion of KIP-477: Add PATCH method for > connector config in Connect REST API. > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API > > There is also a draft PR: https://github.com/apache/kafka/pull/6934. > > Thank you. > > Ivan >
