DanielLeens commented on PR #12661:
URL: https://github.com/apache/seatunnel/pull/12661#issuecomment-6210078870
Thanks for the quick follow-up, @hutiefang76, and thanks @SEZ9 for the
precise list. I re-checked head `a1218f7be` (unchanged since my last review)
against it.
I agree with both of @SEZ9's remaining items, and I would like to tighten my
own earlier stance on the first one. In my review I treated the docs as
optional ("I would not block on that"). Now that the maintainer has asked for
it, please treat it as part of this PR:
1. Behaviour note on the other file-source pages. The same `datetime_format`
option is listed on the EN source pages for BosFile, CosFile, FtpFile,
HdfsFile, ObsFile, OssFile, OssJindoFile, S3File, SftpFile and SmbFile, and
they all read CSV/Text through the same `CsvReadStrategy`/`TextReadStrategy`
path as LocalFile. A one-line pointer to the LocalFile note (or the same short
sentence) on each EN page and its ZH counterpart is enough; if you prefer a
follow-up PR for this, please say so in the thread so SEZ9 can decide.
2. Code comment on the cache replacement. A short comment next to the
`fieldFormatterMap.put(...)` in both `parseTimestampWithFormatterRefresh`
methods (CSV and Text) saying the refresh is exception-driven and single-slot,
so a column that alternates between layouts re-matches on every switch, and
that this is a deliberate trade-off. This also covers my low-severity Issue 1
as a documented decision, so it no longer needs a code change here.
Everything else from my last review still stands: no other source blocker,
and the tests have real failure power.
CI is still the gate. The `Build` check on this head is `ACTION_REQUIRED`,
and I still cannot find an Actions run for `a1218f7be` in your fork. When you
push the two small changes above, that new commit will be a good moment to make
sure Actions is enabled on the fork, so the `Build` workflow runs on the new
head. Thanks again for the patience and care on this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
