DanielLeens commented on PR #12286:
URL: https://github.com/apache/seatunnel/pull/12286#issuecomment-5690123254
Thanks @nzw921rx for the +1, and for re-surfacing Issue 1 from my previous
review.
Agreed it's worth handling, though I'd still call it non-blocking rather
than a merge gate. @goutamadwant the fix is small either way:
- drop `catalogProperties.put("type", "hadoop")` entirely — since
`IcebergCatalogLoader`'s construction is intercepted by `mockConstruction`,
that property is never actually consulted anymore, or
- replace it with something that documents the real setup (`InMemoryCatalog`
via `mockConstruction`), so a future reader doesn't assume a real Hadoop
catalog path is being exercised.
For scope/status: after three review passes I have no other blocking
findings, and CI is fully green (27/27 non-skipped checks, including the two
jobs that were previously red — `windows-latest` unit tests and
`updated-modules-integration-test-part-2`). This nit alone shouldn't hold up
the merge; a quick follow-up commit on this branch works fine, or it can be a
fast-follow after merge if that's easier.
--
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]
