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.

Sign up for GitHub

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

Jump to bottom

Add environment variable handling to Hono example #557

Closed
wants to merge 1 commit into from
Closed

Add environment variable handling to Hono example #557

wants to merge 1 commit into from

Conversation

arjunyel
Copy link

Summary

Add environment variable handling to Hono example

Checklist

  • Added a docs PR that references this PR N/A
  • Added unit/integration tests N/A
  • Added changesets if applicable N/A

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Apr 29, 2024

⚠️ No Changeset found

Latest commit: 778b6d6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jpwilliams jpwilliams mentioned this pull request May 15, 2024
Merged
3 tasks
@jpwilliams
Copy link
Member

Thank you, @arjunyel! 🙌

I've put in a more long-term fix for this that shouldn't require the manual specification of signing/event keys, so I'll close this in favour of the PR at #571.

Would be awesome if you could test that out (npm install inngest@pr-571) and let me know if it also solves your issue. 🙂

@jpwilliams jpwilliams closed this May 15, 2024
jpwilliams added a commit that referenced this pull request May 20, 2024
@jpwilliams
Fix "inngest/hono" serve handler not parsing environment variables (#…
67ca3aa 
…571)

## Summary
<!-- Succinctly describe your change, providing context, what you've
changed, and why. -->

The `"inngest/hono"` serve handler was not fetching environment
variables, which are not reliably accessible via globals in common
environments such as Cloudflare Workers.

In addition, `url` handling was a little wonky and didn't seem to handle
many variants. The hope was that Hono handled that to ensure that
`c.req.url` is always/never absolute, but this doesn't seem to be the
case.

## Checklist
<!-- Tick these items off as you progress. -->
<!-- If an item isn't applicable, ideally please strikeout the item by
wrapping it in "~~"" and suffix it with "N/A My reason for skipping
this." -->
<!-- e.g. "- [ ] ~~Added tests~~ N/A Only touches docs" -->

- [ ] ~Added a [docs PR](https://github.com/inngest/website) that
references this PR~ N/A
- [ ] ~Added unit/integration tests~ N/A
- [x] Added changesets if applicable

## Related
<!-- A space for any related links, issues, or PRs. -->
<!-- Linear issues are autolinked. -->
<!-- e.g. - INN-123 -->
<!-- GitHub issues/PRs can be linked using shorthand. -->
<!-- e.g. "- inngest/inngest#123" -->
<!-- Feel free to remove this section if there are no applicable related
links.-->
- Supersedes #557 as we should no longer be required to manually set
signing and event keys
- Fixes #560's URL issues
- Fixes _some_ of the isues in #556, though there are still some
unknowns for the C++ errors there
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@arjunyel @jpwilliams