fmorillo7694 commented on PR #206: URL: https://github.com/apache/flink-connector-aws/pull/206#issuecomment-5829848928
Thanks @gguptp for the thorough review - every thread is addressed and answered inline. Pushed as four commits on top of `649f4d9`: - **56f13dd** - correctness + API-call reduction: SDK typed exceptions instead of dead HTTP-response checks, single-call database/table resolution (common path is now 1 Get call; fallbacks use paginated `GetTables`/`GetDatabases` metadata with no per-item Gets), pagination in `listViews` and all listing paths, Hive/Spark function class names fall back to JAVA instead of failing, `FakeGlueClient` now emulates paginated responses (+ pagination tests), unused `sts`/`org.json` dependencies removed, SDK/jackson versions inherited from the root pom (the local jackson property had already drifted stale). - **eee3aa0** - schema fidelity: your round-trip question uncovered that watermarks and primary keys were silently lost and computed/metadata columns were corrupted into physical columns. They now round-trip through `flink.schema.*` table parameters (including declared `NOT NULL` and lossy types like `TIMESTAMP(3)`, which Glue's type strings cannot represent), restored at their declared positions on read. Also fixes the view-columns-as-partition-keys bug you spotted and the `DatabaseNotExistException` contract. - **e38ae6c** - configurable AWS client: built via `AWSClientUtil`/`AWSGeneralUtil` like the other connectors, with `aws.*` (FLIP-277 credential modes, `aws.endpoint`) and `http-client.*` catalog options passed through and validated - this also puts `HttpClientOptionUtils` into production use and closes Samrat's config-options ask from the original review. Factory now validates options via `FactoryUtil` (unknown options rejected) with new unit tests; `@PublicEvolving` added. - **37f62bc** - docs: shortcode fix, corrected unsupported-operations list (ALTER TABLE and ADD/DROP PARTITION are supported), concrete Glue quota classes, table-type support note, new `aws.*`/`http-client.*` option docs, and the `content.zh` page. Verification (all tiers, before pushing): 145 unit tests green on the fake backend **and against real AWS Glue** (18 fault-injection skips, eventual-consistency flakes absorbed by reruns), 9 moto wire-level ITCases green, and the credential-gated e2e module 3/3 against real Glue in eu-central-1 - including a new `SHOW CREATE TABLE` fidelity round-trip proving watermark/PK/computed/metadata columns survive the real service. Test account verified clean of leftovers afterwards. Note for whoever can approve CI: the apache-side workflow needs a fresh "Approve and run" on the new head. -- 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]
