-
Notifications
You must be signed in to change notification settings - Fork 594
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
Transform JS SR integration test #21527
Conversation
52de2bb
to
8ed68df
Compare
7d88bd1
to
b2dcd04
Compare
cc @rockwotj - hacked integration test for SR bits |
0b9f079
to
4d92776
Compare
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]>
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 |
4d92776
to
d534177
Compare
force push fix merge conflict on gha workflow |
oh, cool. yeah, I'll knock that out this afternoon. |
d534177
to
2ea0a3e
Compare
"dependencies": { | ||
"@redpanda-data/transform-sdk": "file:../module", | ||
"@redpanda-data/transform-sdk-sr": "file:../sr-module", | ||
"avsc": "github:mtth/avsc#7cb76a6eb7b0d5b5a71c9cfb446acf3eff763006" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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]>
Signed-off-by: Oren Leiman <[email protected]>
2ea0a3e
to
48f4edb
Compare
Working with dependency version hacks:
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.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 supportBlocked by #21491✅Closes CORE-5638
Backports Required
Release Notes