MartijnVisser commented on code in PR #312: URL: https://github.com/apache/flink-connector-kafka/pull/312#discussion_r3989453104
########## AGENTS.md: ########## @@ -0,0 +1,240 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Flink Kafka Connector AI Agent Instructions + +This file provides guidance for AI coding agents working with the Apache Flink Kafka connector codebase. + +## Prerequisites + +- Java 11, 17, or 21. Java 11 syntax must be used everywhere: the build compiles with source level 11 (target 17). On JDK 11 pass `-Pjava11-target`, as CI does. +- Maven 3.8.6 (Maven wrapper `./mvnw` included; prefer it) +- Git +- Docker (every `*ITCase` starts Kafka through Testcontainers) +- Python 3.9 to 3.11 with tox, only for `flink-python` +- Unix-like environment (Linux, macOS, WSL) +- The connector builds against `flink.version` in the root `pom.xml`, which is the lowest supported Flink minor version. A change must also work against every Flink version in `.github/workflows/push_pr.yml` and the `main` rows of `.github/workflows/weekly.yml`. + +## Commands + +### Build + +- Build without tests: `./mvnw clean install -DskipTests` +- Full build with tests: `./mvnw clean verify` +- Build against another Flink version (what CI does): `./mvnw clean install -DskipTests -Dflink.version=<version>` +- Single module: `./mvnw clean install -DskipTests -pl flink-connector-kafka` +- `-Dfast` skips RAT, checkstyle, spotless, enforcer and javadoc, but not japicmp in this repository. +- Dependency convergence (CI runs this on PRs): `./mvnw clean install -DskipTests -Pcheck-convergence -Dflink.convergence.phase=install` + +### Testing + +- `*Test` classes run in the `test` phase, everything else (`*ITCase`) in the `integration-test` phase through a second Surefire execution; there is no Failsafe plugin. +- Single unit test class: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSourceReaderTest` +- Single test method: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSourceReaderTest#testCommitOffsetsWithoutAliveFetchers` +- Single ITCase: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSinkITCase` (`-Dtest` overrides the phase filter; `verify -Dtest=...` runs the class twice) +- ArchUnit rules: `./mvnw test -pl flink-connector-kafka -Dtest='*ArchitectureTest'`, then check `git status flink-connector-kafka/archunit-violations/` (see Testing Standards) +- End-to-end tests need a Flink distribution: `./mvnw clean verify -Prun-end-to-end-tests -DdistDir=<path to flink-<version>>`. CI runs them on every PR. +- PyFlink tests: `./mvnw clean install -DskipTests`, then `cd flink-python && ./dev/lint-python.sh -e mypy,sphinx` (the script is downloaded into `flink-python/dev/` during the Maven `validate` phase) +- CI is defined by `apache/flink-connector-shared-utils` (`.github/workflows/ci.yml@ci_utils`); its Maven command line, including the license check, is the reference when a local run differs from CI. + +### Code Quality + +- Format code: `./mvnw spotless:apply` (skipped automatically on JDK 21; run it on 11 or 17) +- Check formatting: `./mvnw spotless:check` +- Checkstyle: `./mvnw checkstyle:check` +- Checkstyle config: `tools/maven/checkstyle.xml` +- License headers: `./mvnw apache-rat:check` +- `verify` also runs `dependency:analyze` with `failOnWarning=true`: a new import from a transitively available artifact fails the build until the dependency is declared in the module's `pom.xml`. +- japicmp compares against `japicmp.referenceVersion` from the root `pom.xml` and only checks `@Public` API. + +### Documentation + +- There is no docs build in this repository. The Flink docs build (`docs/setup_docs.sh` in `apache/flink`) clones the release branch of this repository and renders `docs/content` and `docs/content.zh`. +- Option tables are hand-written HTML rows; nothing is generated from `ConfigOption` definitions. + +## Repository Structure + +### Modules + +- `flink-connector-kafka` — The connector: `KafkaSource`, `KafkaSink`, `DynamicKafkaSource`, and the Table/SQL factories. Also publishes a test-jar with `KafkaTestEnvironment*` and `testutils`. +- `flink-sql-connector-kafka` — Shaded SQL jar; relocates `org.apache.kafka`. Bundled dependencies are listed in `src/main/resources/META-INF/NOTICE`. +- `flink-connector-kafka-e2e-tests/` — `flink-streaming-kafka-test` (the job), `flink-streaming-kafka-test-base` (shared classes), `flink-end-to-end-tests-common-kafka` (the tests). +- `flink-python` — PyFlink wrappers (`pyflink/datastream/connectors/kafka.py`) and their tests (`pyflink/datastream/connectors/tests/test_kafka.py`). Maven packaging `pom`, no Java. + +### Supporting directories + +- `docs/content/docs/connectors/` and `docs/content.zh/docs/connectors/` — DataStream and Table docs, English and Chinese, same file set. +- `tools/maven/` — checkstyle and suppressions. +- `tools/releasing/shared` — Git submodule with the release scripts (`git submodule update --init`). +- `.github/workflows/` — `push_pr.yml` (PR CI), `weekly.yml` (release branches and Flink snapshots). + +### Key packages in `flink-connector-kafka/src/main/java` + +- `org.apache.flink.connector.kafka.source` — FLIP-27 source. `@PublicEvolving`: `KafkaSource`, `KafkaSourceBuilder`, `enumerator.initializer.OffsetsInitializer`, `enumerator.subscriber.KafkaSubscriber`, `reader.deserializer.KafkaRecordDeserializationSchema`, `metrics.KafkaSourceReaderMetrics`. `@Internal`: `enumerator/`, `reader/`, `split/`, `KafkaSourceOptions`, `enumerator.metadata` (topic integrity). +- `org.apache.flink.connector.kafka.sink` — Sink V2 sink. `@PublicEvolving`: `KafkaSink`, `KafkaSinkBuilder`, `KafkaRecordSerializationSchema` and its builder, `KafkaPartitioner`, `TopicSelector`, `HeaderProvider`, `TransactionNamingStrategy`. `sink.internal` (transactions, producer pool, backchannel) is `@Internal`. +- `org.apache.flink.connector.kafka.dynamic` — Multi-cluster source. `@Experimental`: `DynamicKafkaSource`, `DynamicKafkaSourceBuilder`, `metadata.KafkaMetadataService`, `metadata.KafkaStream`, `metadata.ClusterMetadata`, `metadata.SingleClusterTopicMetadataService`, `KafkaStreamSubscriber`. Enumerator, reader and split classes are `@Internal`. +- `org.apache.flink.connector.kafka.lineage` — OpenLineage facets, `@PublicEvolving`. +- `org.apache.flink.streaming.connectors.kafka.table` — Table/SQL layer. The three factories (`kafka`, `upsert-kafka`, `dynamic-kafka`) are registered in `META-INF/services/org.apache.flink.table.factories.Factory`. `KafkaConnectorOptions` and `DynamicKafkaConnectorOptions` are `@PublicEvolving`; the rest is `@Internal`. +- `org.apache.flink.streaming.util.serialization.JSONKeyValueDeserializationSchema` — `@PublicEvolving`. +- The legacy `FlinkKafkaConsumer` and `FlinkKafkaProducer` exist only on the `v3.x` branches. Do not reintroduce them. + +## Architecture Boundaries + +1. **Source (FLIP-27).** `KafkaSourceEnumerator` runs on the coordinator thread and does all broker I/O through `context.callAsync` (the split lifecycle is documented in its class Javadoc). `KafkaSourceReader` runs on the task thread; the `SplitFetcher` threads own the `KafkaConsumer`, which must only be accessed from one thread. Offset commits are therefore enqueued onto the fetcher by `KafkaSourceFetcherManager.commitOffsets`. A failed commit is logged and counted in the metrics; it never fails the job. +2. **Sink (Sink V2).** `KafkaWriter` runs on the mailbox thread. Producer callbacks run on the Kafka network thread and store the first exception in a `volatile` field that `checkAsyncException()` rethrows on the mailbox thread. `numRecordsSent` is deliberately not volatile; only touch it from the mailbox thread. Review Comment: Correct, and the line had it backwards. `onCompletion` increments `numRecordsSent` on the producer network thread (`KafkaWriter.java:352`), and the only read is the `LOG.warn` just above it, so the field never reaches the mailbox at all. The mailbox constraint belongs to `checkAsyncException()`, whose Javadoc pins it there because the metric counter it increments isn't volatile; I'd conflated the two. Reworded in the latest push. ########## AGENTS.md: ########## @@ -0,0 +1,240 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Flink Kafka Connector AI Agent Instructions + +This file provides guidance for AI coding agents working with the Apache Flink Kafka connector codebase. + +## Prerequisites + +- Java 11, 17, or 21. Java 11 syntax must be used everywhere: the build compiles with source level 11 (target 17). On JDK 11 pass `-Pjava11-target`, as CI does. +- Maven 3.8.6 (Maven wrapper `./mvnw` included; prefer it) +- Git +- Docker (every `*ITCase` starts Kafka through Testcontainers) +- Python 3.9 to 3.11 with tox, only for `flink-python` +- Unix-like environment (Linux, macOS, WSL) +- The connector builds against `flink.version` in the root `pom.xml`, which is the lowest supported Flink minor version. A change must also work against every Flink version in `.github/workflows/push_pr.yml` and the `main` rows of `.github/workflows/weekly.yml`. + +## Commands + +### Build + +- Build without tests: `./mvnw clean install -DskipTests` +- Full build with tests: `./mvnw clean verify` +- Build against another Flink version (what CI does): `./mvnw clean install -DskipTests -Dflink.version=<version>` +- Single module: `./mvnw clean install -DskipTests -pl flink-connector-kafka` +- `-Dfast` skips RAT, checkstyle, spotless, enforcer and javadoc, but not japicmp in this repository. +- Dependency convergence (CI runs this on PRs): `./mvnw clean install -DskipTests -Pcheck-convergence -Dflink.convergence.phase=install` + +### Testing + +- `*Test` classes run in the `test` phase, everything else (`*ITCase`) in the `integration-test` phase through a second Surefire execution; there is no Failsafe plugin. +- Single unit test class: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSourceReaderTest` +- Single test method: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSourceReaderTest#testCommitOffsetsWithoutAliveFetchers` +- Single ITCase: `./mvnw test -pl flink-connector-kafka -Dtest=KafkaSinkITCase` (`-Dtest` overrides the phase filter; `verify -Dtest=...` runs the class twice) +- ArchUnit rules: `./mvnw test -pl flink-connector-kafka -Dtest='*ArchitectureTest'`, then check `git status flink-connector-kafka/archunit-violations/` (see Testing Standards) +- End-to-end tests need a Flink distribution: `./mvnw clean verify -Prun-end-to-end-tests -DdistDir=<path to flink-<version>>`. CI runs them on every PR. +- PyFlink tests: `./mvnw clean install -DskipTests`, then `cd flink-python && ./dev/lint-python.sh -e mypy,sphinx` (the script is downloaded into `flink-python/dev/` during the Maven `validate` phase) Review Comment: You're right, it fails as written. The downloaded scripts land as `-rw-r--r--`, so `./dev/lint-python.sh` gives permission denied; `integration_test.sh` only works because it's the one file in `dev/` that's in git with the bit set. Added `chmod a+x dev/*` to match `python_ci.yml`. Having the build set it instead would need a pom change, since the scripts are fetched over HTTPS and a clean deletes them, so I've left that for a separate ticket rather than widen 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]
