Skip to content

Comments

Migrate all Kinesis ITs to embedded-tests#19046

Open
kfaraz wants to merge 12 commits intoapache:masterfrom
kfaraz:migrate_kinesis_its
Open

Migrate all Kinesis ITs to embedded-tests#19046
kfaraz wants to merge 12 commits intoapache:masterfrom
kfaraz:migrate_kinesis_its

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Feb 23, 2026

Changes

  • Add StreamIngestResource which is implemented by KafkaResource and KinesisResource
  • Add KinesisResource that uses Localstack
  • Convert existing KafkaDataFormatsTest to StreamIndexDataFormatsTest to allow testing different stream types
  • Add KinesisDataFormatsTest and KafkaIndexDataFormatsTest
  • Replace ITKinesisIndexingServiceSerializedTest with KinesisFaultToleranceTest
  • Update SchemaRegistryResource to allow use with Kinesis as well as Kafka
  • Remove ITKinesisIndexingServiceParallelizedTest since Kinesis does not support multi-supervisor yet

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Comment on lines +47 to +49
schema -> schema
.withTimestamp(new TimestampSpec("timestamp", null, null))
.withParserMap(parserMap)

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Builder.withParserMap](1) should be avoided because it has been deprecated.
Comment on lines +64 to +65
DataSchema.builder(baseSpec.getSpec().getDataSchema())
.withParserMap(parserMap)

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Builder.withParserMap](1) should be avoided because it has been deprecated.
@kfaraz kfaraz changed the title Add embedded KinesisDataFormatsTest to replace ITKinesisDataFormatsTest Migrate all Kinesis ITs to embedded-tests Feb 23, 2026
@jtuglu1
Copy link
Contributor

jtuglu1 commented Feb 24, 2026

@kfaraz see #18891. I wonder if I can use your ITs in the PR? This will unblock the upgrade.

@kfaraz
Copy link
Contributor Author

kfaraz commented Feb 24, 2026

Yes, @jtuglu1 , unblocking your PR was the primary motivation behind migrating these tests now. 🙂

Could you help review this PR so that we can merge it off first and then use the tests to verify the changes in your PR?
The tests are currently failing on the CI. I am looking into it.

Meanwhile, you could try running the tests in your PR to catch any bugs.

Copy link
Contributor

@jtuglu1 jtuglu1 left a comment

Choose a reason for hiding this comment

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

LGTM few nits on creating builders for spec/configs like we do in KafkaTuningConfigBuilder, KafkaSupervisorSpecBuilder, and KafkaIOConfigBuilder

protected SupervisorSpec createSupervisorWithParser(String dataSource, String topic, Map<String, Object> parserMap)
{
final KinesisSupervisorSpec baseSpec = createKinesisSupervisorSpec(dataSource, topic, null);
return new KinesisSupervisorSpec(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create KinesisSupervisorSpecBuilder like KafkaSupervisorSpecBuilder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! Let me try including that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants