Pil0tXia commented on code in PR #4837: URL: https://github.com/apache/eventmesh/pull/4837#discussion_r1581865709
########## eventmesh-connectors/eventmesh-connector-http/src/main/resources/sink-config.yml: ########## @@ -26,13 +26,20 @@ pubSubConfig: passWord: httpPassWord connectorConfig: connectorName: httpSink - host: 127.0.0.1 - port: 8987 - path: /test + urls: + - http://127.0.0.1:8987/test ssl: false - webhook: false keepAlive: true keepAliveTimeout: 60000 idleTimeout: 5000 # timeunit: ms, recommended scope: common(5s - 10s), webhook(15s - 60s) connectionTimeout: 5000 # timeunit: ms, recommended scope: 5 - 10s - maxConnectionPoolSize: 5 \ No newline at end of file + maxConnectionPoolSize: 5 + retryConfig: + maxAttempts: 3 + interval: 1000 + retryAll: false + webhookConfig: + activate: false + exportPath: /export + port: 8988 + idleTimeout: 5000 Review Comment: I saw your `SinkConnectorConfig#populateFieldsWithDefaults()` method, which is a good design. The `connectorConfig.webhookConfig.idleTimeout` here is for the server, shall we differ its naming from the `connectorConfig.idleTimeout` at L34? ########## eventmesh-connectors/eventmesh-connector-http/src/main/resources/sink-config.yml: ########## @@ -26,13 +26,20 @@ pubSubConfig: passWord: httpPassWord connectorConfig: connectorName: httpSink - host: 127.0.0.1 - port: 8987 - path: /test + urls: + - http://127.0.0.1:8987/test ssl: false - webhook: false keepAlive: true keepAliveTimeout: 60000 idleTimeout: 5000 # timeunit: ms, recommended scope: common(5s - 10s), webhook(15s - 60s) connectionTimeout: 5000 # timeunit: ms, recommended scope: 5 - 10s - maxConnectionPoolSize: 5 \ No newline at end of file + maxConnectionPoolSize: 5 + retryConfig: + maxAttempts: 3 + interval: 1000 + retryAll: false + webhookConfig: + activate: false + exportPath: /export + port: 8988 + idleTimeout: 5000 Review Comment: I saw your `SinkConnectorConfig#populateFieldsWithDefaults()` method, which is a good design. The `connectorConfig.webhookConfig.idleTimeout` here is for the server, shall we differ its naming from the `connectorConfig.idleTimeout` at L34? -- 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: issues-unsubscr...@eventmesh.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@eventmesh.apache.org For additional commands, e-mail: issues-h...@eventmesh.apache.org
