You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "mmodzelewski (via GitHub)" <gi...@apache.org> on 2025/11/14 15:26:32 UTC

[PR] feat(connectors): extend runtime API with config management options [iggy]

mmodzelewski opened a new pull request, #2348:
URL: https://github.com/apache/iggy/pull/2348

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2535067601


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,9 +164,46 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub is_active: bool,
+}
+
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersions {
+    pub sinks: HashMap<String, Vec<ConnectorConfigVersionInfo>>,

Review Comment:
   I was wondering, whether it should return all existing versions, or just the active ones?



##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,9 +164,46 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub is_active: bool,
+}

Review Comment:
   Maybe let's also include some sort of timestamp/created_at field - for the file config provider, this could be taken from the file metadata itself.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2538172838


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,17 +174,57 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub created_at: DateTime<Utc>,
+}
+
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersions {
+    pub sinks: HashMap<String, ConnectorConfigVersionInfo>,
+    pub sources: HashMap<String, ConnectorConfigVersionInfo>,
+}
+
 #[async_trait]
-pub trait ConnectorsConfigProvider {
-    async fn load_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+pub trait ConnectorsConfigProvider: Send + Sync {

Review Comment:
   I pushed the changes with delete implemented



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528270187


##########
core/connectors/runtime/README.md:
##########
@@ -71,9 +71,19 @@ Currently, it does expose the following endpoints:
 - `GET /health`: health status of the runtime.
 - `GET /sinks`: list of sinks.
 - `GET /sinks/{key}`: sink details.
-- `GET /sinks/{key}/config`: sink config, including the optional `format` query parameter to specify the config format.
+- `GET /sinks/{key}/configs`: list of configuration versions for the sink.
+- `POST /sinks/{key}/configs`: add a new configuration version for the sink.
+- `GET /sinks/{key}/configs/{version}`: configuration details for a specific version.
+- `GET /sinks/{key}/active_config`: active configuration details.

Review Comment:
   Maybe let's always use `/configs` path, and then we could have `/active` and `/plugin` for these 2 other cases?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2536848906


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,9 +164,46 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub is_active: bool,
+}
+
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersions {
+    pub sinks: HashMap<String, Vec<ConnectorConfigVersionInfo>>,

Review Comment:
   Yes, only the active ones are good enough.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2536613807


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -59,30 +59,84 @@ impl Default for ConnectorConfig {
     }
 }
 
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct CreateSinkConfigCommand {

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528453738


##########
core/connectors/runtime/README.md:
##########
@@ -71,9 +71,19 @@ Currently, it does expose the following endpoints:
 - `GET /health`: health status of the runtime.
 - `GET /sinks`: list of sinks.
 - `GET /sinks/{key}`: sink details.
-- `GET /sinks/{key}/config`: sink config, including the optional `format` query parameter to specify the config format.
+- `GET /sinks/{key}/configs`: list of configuration versions for the sink.
+- `POST /sinks/{key}/configs`: add a new configuration version for the sink.
+- `GET /sinks/{key}/configs/{version}`: configuration details for a specific version.
+- `GET /sinks/{key}/active_config`: active configuration details.

Review Comment:
   yeah, makes sense, I'll update it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2535065768


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -59,30 +59,84 @@ impl Default for ConnectorConfig {
     }
 }
 
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct CreateSinkConfigCommand {

Review Comment:
   Maybe let's just call it `CreateSinkConfig` (the name is already an intent), same for the source one.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2536618474


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,9 +164,46 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub is_active: bool,
+}
+
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersions {
+    pub sinks: HashMap<String, Vec<ConnectorConfigVersionInfo>>,

Review Comment:
   Yeah, probably active ones would be enough if we want to use it just for checking with what's currently in the runtime. I'll update it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528551853


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -111,8 +111,20 @@ pub struct StreamProducerConfig {
 }
 
 #[async_trait]
-pub trait ConnectorsConfigProvider {
-    async fn load_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+pub trait ConnectorsConfigProvider: Send + Sync {
+    async fn get_all_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+    async fn get_sink_configs(&self, key: &str) -> Result<Vec<SinkConfig>, RuntimeError>;
+    async fn get_sink_config(

Review Comment:
   Updated. In the API version is still required though, as we need a distinction from get all versions endpoint



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski merged PR #2348:
URL: https://github.com/apache/iggy/pull/2348


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528278801


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -111,8 +111,20 @@ pub struct StreamProducerConfig {
 }
 
 #[async_trait]
-pub trait ConnectorsConfigProvider {
-    async fn load_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+pub trait ConnectorsConfigProvider: Send + Sync {
+    async fn get_all_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+    async fn get_sink_configs(&self, key: &str) -> Result<Vec<SinkConfig>, RuntimeError>;
+    async fn get_sink_config(

Review Comment:
   Let's make version optional (if not provided, the latest version should be returned).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528272373


##########
core/connectors/runtime/src/api/sink.rs:
##########
@@ -38,8 +39,11 @@ pub fn router(state: Arc<RuntimeContext>) -> Router {
     Router::new()
         .route("/sinks", get(get_sinks))
         .route("/sinks/{key}", get(get_sink))
-        .route("/sinks/{key}/config", get(get_sink_config))
+        .route("/sinks/{key}/plugin_config", get(get_sink_plugin_config))
         .route("/sinks/{key}/transforms", get(get_sink_transforms))
+        .route("/sinks/{key}/configs", get(get_sink_configs))
+        .route("/sinks/{key}/configs/{version}", get(get_sink_config))
+        .route("/sinks/{key}/active_config", get(get_sink_active_config))

Review Comment:
   Same remark here with the endpoint paths.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2528277020


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -111,8 +111,20 @@ pub struct StreamProducerConfig {
 }
 
 #[async_trait]
-pub trait ConnectorsConfigProvider {
-    async fn load_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+pub trait ConnectorsConfigProvider: Send + Sync {

Review Comment:
   We can also add one more function (as described previously) that would return all existing configs versions (e.g. could be a HashMap or Vec of some simple struct) - this would be useful for config updates checking in the background job (much less data to transfer over the wire).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2537807832


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,17 +174,57 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub created_at: DateTime<Utc>,
+}
+
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersions {
+    pub sinks: HashMap<String, ConnectorConfigVersionInfo>,
+    pub sources: HashMap<String, ConnectorConfigVersionInfo>,
+}
+
 #[async_trait]
-pub trait ConnectorsConfigProvider {
-    async fn load_configs(&self) -> Result<ConnectorsConfig, RuntimeError>;
+pub trait ConnectorsConfigProvider: Send + Sync {

Review Comment:
   I think we'll need also need the delete functions to remove unused configurations, and that should be all :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(connectors): extend runtime API with config management options [iggy]

Posted by "mmodzelewski (via GitHub)" <gi...@apache.org>.
mmodzelewski commented on code in PR #2348:
URL: https://github.com/apache/iggy/pull/2348#discussion_r2536622244


##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -110,9 +164,46 @@ pub struct StreamProducerConfig {
     pub linger_time: Option<String>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ConnectorConfigVersionInfo {
+    pub version: u64,
+    pub is_active: bool,
+}

Review Comment:
   Ok, I'll add it 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org