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

Wasm example improvements #12

Merged
merged 7 commits into from
Apr 18, 2022

Conversation

vuldin
Copy link
Member

@vuldin vuldin commented Mar 24, 2022

This PR contains a few updates:

  • dependencies are updated for the wasm project
  • Multiple updates to three READMEs: 1) main, 2) the JS client, and 3) wasm app
  • switch to avsc mainly due to details found here

The updated README contains steps for running the app: https://github.com/vuldin/redpanda-examples/blob/wasm-example-improvements/wasm/js/transform_avro/README.md

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2022

CLA assistant check
All committers have signed the CLA.

@vuldin
Copy link
Member Author

vuldin commented Mar 24, 2022

This PR resolves an error when attempting to run npm run build with node@17:

  this[kHandle] = new _Hash(algorithm, xofLen);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:67:19)
    at Object.createHash (node:crypto:135:10)
    at module.exports (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/webpack/lib/util/createHash.js:135:53)
    at NormalModule._initBuildHash (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/webpack/lib/NormalModule.js:417:16)
    at handleParseError (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/webpack/lib/NormalModule.js:471:10)
    at /Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/webpack/lib/NormalModule.js:503:5
    at /Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/webpack/lib/NormalModule.js:358:12
    at /Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/loader-runner/lib/LoaderRunner.js:373:3
    at iterateNormalLoaders (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/loader-runner/lib/LoaderRunner.js:214:10)
    at Array.<anonymous> (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/loader-runner/lib/LoaderRunner.js:205:4)
    at Storage.finished (/Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:55:16)
    at /Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:91:9
    at /Users/josh/projects/redpanda/redpanda-examples/wasm/js/transform_avro/node_modules/graceful-fs/graceful-fs.js:123:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3) {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

@vuldin
Copy link
Member Author

vuldin commented Mar 25, 2022

I pushed an update to the switch avro-js for avsc commit, which changed the output topic name to match what was referenced in the README (avro).

@vuldin vuldin force-pushed the wasm-example-improvements branch 2 times, most recently from d175b3e to 3a5cb08 Compare April 3, 2022 19:38
@vuldin
Copy link
Member Author

vuldin commented Apr 3, 2022

Pushed an update that makes this PR compatible with another PR in review #18. Once reviewed/approved, either PR can now be merged first.

Copy link

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

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

Really nice work, thanks for the contribution! Only non nit comment from me was to keep the version consistent with the technical preview release.

wasm/js/transform_avro/package.json Outdated Show resolved Hide resolved
wasm/js/transform_avro/README.md Outdated Show resolved Hide resolved
wasm/js/transform_avro/src/main.js Show resolved Hide resolved
@vuldin
Copy link
Member Author

vuldin commented Apr 8, 2022

Added a new commit that addressed feedback (thanks @graphcareful !). I also realized that upload-schema.js was included in ESM module format. This was done because another PR adds a dependency that only supports ESM modules and producer.js is refactored there. But since that PR isn't merged I'll wait to add upload-schemas.js until then.

@vuldin
Copy link
Member Author

vuldin commented Apr 8, 2022

Let me know if anyone thinks I should squash the commits and modify them so they make more sense based on what is changing in this PR.

Copy link
Collaborator

@jrkinley jrkinley left a comment

Choose a reason for hiding this comment

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

Thanks for this @vuldin. My only comment is I think we should consolidate all Docker Compose examples in the same top-level directory. Happy to do it in this PR or a follow up PR.

clients/js/README.md Outdated Show resolved Hide resolved
- use wasm version of wasm-api client lib
- switch to port 9092 (and similar ports for schema, pandaproxy, prometheus) for local/external connectivity
- update README.md
- remove upload-schemas.js (will be included in subsequent PR)
- move compose.yaml and redpanda.yaml into docker-compose directory (and rename files)
- rebase from main branch (handle merge conflicts)
- update docs to reflect updates from previous commits to main
- add cleanup section to README
@vuldin
Copy link
Member Author

vuldin commented Apr 12, 2022

I've updated the branch with the following changes:

  • move compose.yaml and redpanda.yaml into docker-compose directory (and rename files)
  • rebase from main branch (handle merge conflicts)
  • update docs to reflect updates from previous commits to main
  • add cleanup section to wasm README
  • update js client README to reflect docker compose file changes

@vuldin
Copy link
Member Author

vuldin commented Apr 12, 2022

I don't like the commit history... I would like to have fixed that prior to a PR. But this PR has been around a while and review has already started, so I'm ok with keeping it as-is if everyone else is.

@vuldin vuldin requested a review from jrkinley April 13, 2022 02:39
@vuldin vuldin mentioned this pull request Apr 15, 2022
@jrkinley jrkinley merged commit 1e7b676 into redpanda-data:main Apr 18, 2022
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.

None yet

4 participants