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

Transform JS SR integration test #21527

Merged

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Jul 19, 2024

Working with dependency version hacks:

  • One is WIP on upstream testcontainers-go
    • testcontainers-go PR is merged, but I think we're still effectively blocked on a version bump. we could pin to the commit we need on upstream main, but that's not particularly better than pinning to my fork.
  • The other is a known issue with avsc. The fix is in their main but not yet released. I have a backport PR open, but no activity there.
    • NOTE(oren): pointed this straight to HEAD of avsc/master as of 8.22.2024. Best of bad alternatives at this time.

As those issues resolve, we can unwind the DNM commits and should be left with a JS/SR integration test fit for dev.

Non-working but workable integration test for Javascript SR support

Blocked by #21491

Closes CORE-5638

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@oleiman oleiman self-assigned this Jul 19, 2024
@github-actions github-actions bot added area/build area/wasm WASM Data Transforms labels Jul 19, 2024
@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch 9 times, most recently from 52de2bb to 8ed68df Compare July 24, 2024 18:11
@oleiman oleiman marked this pull request as ready for review July 24, 2024 18:36
@oleiman oleiman requested a review from a team as a code owner July 24, 2024 18:36
@oleiman oleiman requested review from hobbseltoff and removed request for a team July 24, 2024 18:36
@oleiman oleiman marked this pull request as draft July 24, 2024 18:36
@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch 4 times, most recently from 7d88bd1 to b2dcd04 Compare July 24, 2024 19:33
@oleiman
Copy link
Member Author

oleiman commented Jul 24, 2024

cc @rockwotj - hacked integration test for SR bits

@oleiman oleiman requested a review from rockwotj July 24, 2024 19:41
@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch from 0b9f079 to 4d92776 Compare August 22, 2024 20:36
@oleiman
Copy link
Member Author

oleiman commented Aug 22, 2024

force push bumps test containers and squashes up some dependent commits

And redpanda module. To grab support for arbitrary bootstrap configs.

Also removes usage of deprecated methods and makes some minor tweaks
to account for new version.

Signed-off-by: Oren Leiman <[email protected]>
This is mostly for JavaScript, but shouldn't have any ill effect
on other SDKs.

Signed-off-by: Oren Leiman <[email protected]>
@rockwotj
Copy link
Contributor

BTW we should be able to point our package.json for the buffer repo directly to the main branch on github, that should be good enough to unblock this

@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch from 4d92776 to d534177 Compare August 22, 2024 20:44
@oleiman
Copy link
Member Author

oleiman commented Aug 22, 2024

force push fix merge conflict on gha workflow

@oleiman
Copy link
Member Author

oleiman commented Aug 22, 2024

BTW we should be able to point our package.json for the buffer repo directly to the main branch on github, that should be good enough to unblock this

oh, cool. yeah, I'll knock that out this afternoon.

@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch from d534177 to 2ea0a3e Compare August 22, 2024 22:50
"dependencies": {
"@redpanda-data/transform-sdk": "file:../module",
"@redpanda-data/transform-sdk-sr": "file:../sr-module",
"avsc": "github:mtth/avsc#7cb76a6eb7b0d5b5a71c9cfb446acf3eff763006"
Copy link
Member Author

Choose a reason for hiding this comment

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

npm doesn't seem to accept a branch name on its own - has to be either a commit hash or a tag I guess?

Copy link
Contributor

Choose a reason for hiding this comment

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

if you drop it then it defaults to the main branch. I am fine pinning - either way

Copy link
Member Author

Choose a reason for hiding this comment

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

what npm version are you using? whenever I try to install this package from a branch (no tag or commit hash), i trip over this, which is supposedly fixed, but I haven't had the patience to really look hard at it.

$ npm --version
10.8.1

Copy link
Contributor

Choose a reason for hiding this comment

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

This is just from memory. Don't worry about it - this is fine 👍

Depends on latest avsc, which is unreleased. In the next commit we
point directly to the head of avsc master branch as of 2024.08.22.

Signed-off-by: Oren Leiman <[email protected]>
The main branch has some unreleased improvements that we need
to run in a wasm env. When (if) avsc v6 drops, we can remove
this and point the npm repos.

Signed-off-by: Oren Leiman <[email protected]>
@oleiman oleiman force-pushed the xform/sdk/core-5638/js-sr-integration-test branch from 2ea0a3e to 48f4edb Compare August 22, 2024 23:10
@oleiman oleiman marked this pull request as ready for review August 22, 2024 23:19
@oleiman oleiman changed the title DNM: Transform JS SR integration test Transform JS SR integration test Aug 22, 2024
@oleiman oleiman merged commit 49adab7 into redpanda-data:dev Aug 23, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build area/wasm WASM Data Transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants