fmorillo7694 commented on PR #206: URL: https://github.com/apache/flink-connector-aws/pull/206#issuecomment-5967206839
@Samrat002 thanks, all five are real and all five are fixed in 5e057b2 (one commit on top of the squash, so your single commit request still holds; I can fold it in whenever you prefer). Point by point: **1. Sync HTTP client option precedence (AWSGeneralUtil).** Confirmed. `generic.merge(syncSpecific)` let the generic map win because `AttributeMap.merge` keeps the receiver's value for overlapping keys, so a legacy `aws.http-client.max-concurrency` or read timeout silently beat `http-client.apache.max-connections` / `http-client.socket-timeout-ms` when both were set. The merge is now the other way round in a new `getSyncHttpClientConfiguration`, with the rule documented in its javadoc and in the Glue docs. Two tests: both set, the Apache specific option wins; only the generic ones set, they still apply. I verified the both set test fails against the old order (legacy 100 beat specific 7). You are right that this is shared code; the Kinesis source and DynamoDB streams source call the same overload, so they get the fix too. **2. restoreSchema declaredType branch.** Confirmed. A column with a physical type override (for example `TIMESTAMP(3)`, stored in Glue as plain `timestamp`) that another engine dropped was resurrected from the recorded override. The branch now has the same `glueColumns.containsKey` guard as the plain branch, so the column is omitted and warned like any other dropped column. New test creates such a table, drops the column through `UpdateTable` as another engine would, and asserts the Flink schema omits it; it fails on the old code. **3. Comma joined metadata.** Confirmed. `column-order`, `not-null-columns` and `primary-key.columns` are now written with an escaping join (`,` and `\` inside a name become `\,` and `\\`) and read with the matching split. Values written before escaping contain neither character, so they decode byte for byte as before (tested). New tests: a round trip property over separator and escape bearing names, and a catalog level test with columns named `first,second` and `back\slash` that keeps order, NOT NULL and the primary key intact. **4. ConnectorRegistry dead entries.** Confirmed. `extractTableLocation` records a location only when the value contains `://`, which a Kinesis ARN, Kafka bootstrap servers, a DynamoDB table name, an HBase quorum or a Hive conf dir never do. Those five entries are removed; jdbc, filesystem, elasticsearch, opensearch and mongodb stay. The registry test now feeds each registered key a realistic value through `extractTableLocation` and asserts it becomes the location, so a non URI entry cannot creep back in. The docs now state the storage location rule (URI options only, synthetic `flink://db/table` for partitioned tables of non URI connectors because Glue refuses `CreatePartition` without one). **5. CLIENT_TYPE_URLCONNECTION.** Confirmed dead; only `apache` is wired and the Glue factory test asserts `urlconnection` is rejected. Constant removed. On your earlier questions about `stored.rules`, `archunit.properties` and moto, I replied inline on each. Verification: aws-base 104/104, flink-catalog-aws-glue 971 unit (plus 10 moto ITCases), spotless and checkstyle clean on both modules, fork CI running on 5e057b2. -- 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]
