featzhang commented on PR #30:
URL:
https://github.com/apache/flink-connector-http/pull/30#issuecomment-4367924489
Hi @davidradl, friendly ping — this PR has been reshaped to follow your
original feedback ("don't duplicate the readme — put things into `docs/`, in
the style we use for Kafka"). All five review threads are now marked resolved /
outdated, and the current HEAD (`ee45277`) only touches the documentation
folder:
```
docs/content.zh/docs/connectors/table/http.md | 88 +++++++++++++++-------
docs/content/docs/connectors/table/http.md | 89 +++++++++++++++++-----
2 files changed, 169 insertions(+), 8 deletions(-)
```
Summary of changes since your review:
| Previous concern | Current state |
|------------------|---------------|
| *"put examples into `docs/`, not the README"* | No README change at all;
Quick Start lives under `docs/content*/docs/connectors/table/http.md`. |
| *"connector name is not correct"* | Sink DDLs use `'connector' =
'http-async-sink'` (matches `HttpDynamicTableSinkFactory.IDENTIFIER`); lookup
DDLs use `'connector' = 'http'` (matches `HttpLookupTableSourceFactory`). |
| *"don't document options twice — they could drift"* | Quick Start
intentionally shows only the minimal required options (`url`, `format`,
`insert-method`); the existing full option tables remain the single source of
truth. Quick Start ends with a pointer to those sections. |
| *"please check the examples work"* | Verified locally: SQL options
(`format`, `url`, `insert-method`) match
`HttpDynamicTableSinkFactory.requiredOptions()` /
`HttpLookupTableSourceFactory.requiredOptions()`; the DataStream example
compiles against the current `HttpSink.builder()` /
`HttpSinkRequestEntry(String, byte[])` API. |
| *"include a unit test to drive the sample"* | Happy to add a tiny
compile-only smoke test if you still want one — let me know and I'll push it. |
GitHub Actions (Java 11 + 17) is green, PR is `MERGEABLE`. Could you take
another look when you have a moment?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
