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

Re-enable disabled JOIN tests. #3364

Closed
big-andy-coates opened this issue Sep 17, 2019 · 3 comments · Fixed by #3550
Closed

Re-enable disabled JOIN tests. #3364

big-andy-coates opened this issue Sep 17, 2019 · 3 comments · Fixed by #3550
Milestone

Comments

@big-andy-coates
Copy link
Contributor

We have tests disabled in joins.json waiting on KIP-479.

This is a blocker for the next release.

@big-andy-coates
Copy link
Contributor Author

These tests still fail if re-enabled. They are expecting an internal topic name of _confluent-ksql-some.ksql.service.idquery_CSAS_LEFT_OUTER_JOIN_0-KSTREAM-JOINTHIS-0000000008-store-changelog but actual is _confluent-ksql-some.ksql.service.idquery_CSAS_LEFT_OUTER_JOIN_0-Join-this-join-store-changelog.

This would be a breaking change for our existing users.

@big-andy-coates
Copy link
Contributor Author

Looks like @agavra 'fixed' the expected topologies in commit 9f4dd9c. We'll need to revert this and get all tests passing again if we're to avoid releasing a breaking change.

@bbejeck
Copy link
Contributor

bbejeck commented Oct 11, 2019

But we don't want to revert all the changes as with KIP-307 Grouped now names the aggregate processor and the new StreamJoined names the join processors. These are non-breaking changes for the user and should remain in the topology files. I'll have a PR soon that will re-enable the join tests and disable the naming of join state stores.

bbejeck added a commit that referenced this issue Oct 11, 2019
…s from StreamJoined, re-enable join tests. (#3550)

This PR disables the naming of stores in join operations and reverts the named stores in the expected topologies files. Note that in the future KSQL can easily re-enable the naming of state stores for joins. After this PR is merged, we should be able to close #3364

Reviewers: Andy Coates <[email protected]>, Almog Gavra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants