Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eventub connector to support multiple kcqls #1108

Merged

Conversation

GoMati-MU
Copy link
Contributor

@GoMati-MU GoMati-MU commented Apr 5, 2024

Description

This one is to allow multi-statement KCQL support with additional features mentioned:

  • check input and output topic names against regex
  • disallow one-to-many mappings

Testing

  1. Tests were changed around the changed parts of the code (e.g. Source Record mapping)
  2. Additional unit tests were added

@stheppi
Copy link
Contributor

stheppi commented Apr 5, 2024

  • disallow many-to-one mappings

I imagine you mean one-to-manny mapping :)

} catch (ConfigException e) {
assertEquals(outputErrorMessage, e.getMessage());
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there needs to be a few more tests:

INSERT INTO a SELECT * FROM x;
INSERT INTO a SELECT * FROM y;

should be accepted

INSERT INTO a SELECT * FROM x;
INSERT INTO b SELECT * FROM x;

should not be accepted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added those scenarios, thanks!

Copy link
Contributor

@stheppi stheppi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comments and also the info on connector source start method where validation should happen.

@GoMati-MU
Copy link
Contributor Author

Addressed rest of the comments

@GoMati-MU GoMati-MU merged commit 39a1eec into feat/java-azure-connector Apr 8, 2024
146 of 147 checks passed
@GoMati-MU GoMati-MU deleted the feat/eventub-support-multiple-kcqls branch April 8, 2024 08:09
@GoMati-MU GoMati-MU restored the feat/eventub-support-multiple-kcqls branch April 8, 2024 10:04
@GoMati-MU GoMati-MU deleted the feat/eventub-support-multiple-kcqls branch April 8, 2024 10:04
GoMati-MU added a commit that referenced this pull request Apr 23, 2024
* adding Azure EventHubs connector skeleton with config

* Azure EventHubs Connector MVP (#1033)

Adds Azure EventHubs Connector MVP over Kafka API.

Commits:

* Azure Connector MVP

* added prefix to Config

* adressing PR comments pt. 1

* adressing PR comments pt. 2

* addressing PR comments pt. 3

* removed unused files

* addressing PR comments pt. 4

* small log issue, getting rid of unused singleton classes

* Build KCQL from gradle (#1042)

* adding Gradle buildscripts for KCQL

* LC-128 adding KCQL to Azure EventHubs and additional properties to Azure

* removing mongodb module (it's scrapped)

* added org.antlr.* package shadowing to avoid conflicts in kafka connect

* adressing PR comments, fixing unit tests

* Azure Eventhubs bytes passthrough (#1047)

* Abstracting data that's being passed through connector to Bytes

* cleanup, additional tests

* picking the first KCQL (in case there are more)

* delegated KCQL properties handling to ProducerProvider

* PR comments

* PR comments pt.2

* added java-specific .gitignore

* missing import

* renaming producer to better indicate its purpose

* Lc 136 eventhubs get output input topics from kcql (#1058)

* getting input/output from KCQL initial impl

* removing small issues (we don't need deserializers configs)

* codacy check

* addressing PR comments pt.1

* cleanup of multiple output topics

* remove 'java' part from package name (#1103)

* remove 'java' part from package name

* comments and remarks pt. 1

* eventub connector to support multiple kcqls (#1108)

* support multiple KCQLs

* remove 'java' part from package name

* merged latest changes, added multi-KCQL support

* fixing bug aroud regex check for topic name, added unit tests

* build scripts (in order to merge PRs)

* additional tests

* Added KCQL parsing and validation to Connector class

* addressing PR comments pt. 1

* Using connector name as consumer group (#1109)

* Using connector name as consumer group

* not exposing ignored ConsumerProperties in ConfigDef

not exposing ignored ConsumerProperties in ConfigDef (as connector sets
it by itself)

* file movements

* GitHub actions for java connectors (#1147)

* preparing modules list in Gradle

* addung workflow for build scripts

* adding Java release pipeline, small regex changes

* JDK to 17 for full-support

* pushing version to 6.4.0-SNAPSHOT to keep them in sync with sbt

* add dependency check to java build workflow

* removed KCQL from java release (as it's built by sbt)

* removed unnecessary parameter

* fix dependency check

* push java test results to subdirectory

* run build on push branch

* run actions on push

* remove build workflow run on every push

* fix github warnings for deprecated functions

* addressing java-matrix output being reset with scala artifacts

* creating separate output from build script 'java_modules'

* Feat/java license headers (#1160)

* add java headers config

* add license headers to source files

* add missing HEADER file

* this commit should fail (no header in one of the tests)

* fixing commit plus including info about check in github actions

* addressing PR comments pt. 1

* PR comments pt.2

* added assertj dependancy for unit tests

* fix build script

* fixed failing test

* addressing PR comments pt.3

* adressed PR comments pt.4

* adressed PR comments pt.5

* Refactoring for simplicity (#1170)

* Refactoring for simplicity
Method Extraction: Extracted repetitive setup code into helper methods like mockKeyValueTypes, mockByteBlockingProducer, mockConsumerRecord, mockEmptyHeaders, and mockRecordsQueue.
Parameterized Methods: Made the mockRecordsQueue method varargs to accept variable numbers of input topics for better reusability.
Local Variable Removal: Removed unnecessary local variables like emptyHeaderList and directly invoked Collections.emptyIterator() where needed.
Inline Initialization: Initialized recordsQueue directly in the test methods instead of setting it up in setUp method, as it's only used within the scope of those methods.
Stream Processing: Utilized Java Streams to simplify the creation of consumerRecordList in the mockRecordsQueue method.
Simplified Assertions: Removed unnecessary assertions and verifications to reduce clutter and improve readability.

* small optimizations (imports, constants, formatting)

---------

Co-authored-by: Mati Urban <[email protected]>

---------

Co-authored-by: Mati Urban <[email protected]>
Co-authored-by: David Sloan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants