DanielLeens commented on PR #12384:
URL: https://github.com/apache/seatunnel/pull/12384#issuecomment-5861178270
Thanks for the update, @TianHengZhuang, and for confirming that `Build` is
green - I can see it is `success` on `7d07199584`, and there are no conflicts
with `dev`.
I want to make sure the four points from my last review are tracked
accurately, because I could not find them in the branch yet. The only commit on
top of `5edc05cd59` (the head I reviewed) is `7d07199584` ("chore: retrigger
CI"), and it changes no files. I also diffed the PR's own patch between
`5edc05cd59` and the current head and it is identical. Concretely, on the
current head:
1. `batch_size`: the `Conditions.greaterThan(BATCH_SIZE, 0)` constraint is
in `InfluxDBSinkFactory.java`, but I don't see a `docs/en` / `docs/zh` note or
an `incompatible-changes.md` entry - the PR touches only
`InfluxDBSinkFactory.java` and `InfluxDBFactoryTest.java`.
2. Docs: no `docs/en` or `docs/zh` changes are in the PR yet.
3. `write_timeout`:
`Conditions.greaterThan(InfluxDBSinkOptions.WRITE_TIMEOUT, 0)` is still present
in `InfluxDBSinkFactory.java`.
4. Tests: `InfluxDBFactoryTest.java` is unchanged since the head I reviewed.
It is possible the fixes were committed locally (or pushed to a different
branch) and did not reach this PR's branch - a quick `git log origin/<branch>`
against the PR head would confirm. Once the docs / incompatible-changes entry
and the `write_timeout` decision are pushed, I'm happy to take another full
look. Thank you for your patience - this one is close!
--
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]
