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

Add an example for Node.js #252

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add an example for Node.js #252

wants to merge 5 commits into from

Conversation

BjrInt
Copy link
Member

@BjrInt BjrInt commented Nov 22, 2022

This example is based off the FastAPI example with some differences:

  • The state example is using an ephemeral keypair (used to send message) instead of a counter
  • The "post a message on Aleph" and "retrieve http post data" are merged into a single example
  • There is no "cache"

This example is based off the FastAPI example.
@BjrInt BjrInt requested a review from hoh November 22, 2022 14:06
@hoh
Copy link
Member

hoh commented Nov 22, 2022

The goal of the state example is to mute data in a file on disk inside a mutable volume. Why change its logic ?

@hoh
Copy link
Member

hoh commented Nov 22, 2022

Posting a message on Aleph and handling HTTP Post requests are two different things, we should keep them separate in order to test and diagnose better.

@hoh
Copy link
Member

hoh commented Nov 22, 2022

How should we build and package this application in the CI ?

@moshemalawach
Copy link
Member

Not reviewed at all, this was a spam comment by a spam account @NostalgicGareth. He goes and spam a lot of projects like this.

oksome
oksome approved these changes Dec 1, 2022
oksome
oksome approved these changes Dec 1, 2022
@BjrInt BjrInt linked an issue Dec 10, 2022 that may be closed by this pull request
Copy link
Member

@hoh hoh left a comment

Choose a reason for hiding this comment

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

Forgot to submit this review 🫣

})
})

app.get('/env', (_req, res) => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add docstrings or comments to explain what each function is doing ?

app.get('/raise', () => {
try {
throw new Error('This error was raised on purpose')
} catch (error) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is the error catched ?

try {
console.time('Fetching message')
const posts = await post.Get({
APIServer: "https://api2.aleph.im",
Copy link
Member

Choose a reason for hiding this comment

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

The API server is passed as environment variable to the VM, and the SDK should use that value by default.


addToReqStatus(data.error)
})
</script>
Copy link
Member

Choose a reason for hiding this comment

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

Should we use this page in the Python example as well ?

@MHHukiewitz
Copy link
Member

Can we get confirmation that this still works as intended?

We talked about how we should support more languages/frameworks in the VMs and also to improve examples/docs for deploying these. Merging this PR should help with that.

@hoh hoh added the wip Work in progress... label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip Work in progress...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Typescript SDK is not compatible with the runtime Nodejs 12
5 participants