-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: Pluggable Management Api #11
base: master
Are you sure you want to change the base?
feat: Pluggable Management Api #11
Conversation
Totally agree! I think there was some kind of confusion that a lot of these websocket context attributes are optional (source) |
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.
@reconbot this is great - thanks for getting it in!
A few small suggestions below - if they're not clear or you just don't have time, let me know and I'll merge this as-is 👍
Unless you got big objections I'd be happy to have this merge as is. This change lets me drop my fork ;-) |
7466676
to
188a7d6
Compare
I pushed a type check fix - opened #12 to run ci on prs |
Ahh I forgot my own todo items! I'll get on that |
Pushed some docs
What do you think? It seems to work as is, so I'm alright with it. |
6091b97
to
5fa1884
Compare
Let me know what you want to do with the closures. I pushed the types. |
Merged a few of your PRs - will get back to this one some time this week 💯 |
Ftr this is working well on my fork |
Say you want to use a custom endpoint for your management api communication. Say you want specific configuration? Well now you can! By providing an already constructed management api class. If you don't want to, no problem, we'll assume your endpoint for you from the incoming websocket event. - Added optional ServerArgs.apiGatewayManagementApi - Refactored Event types to use a APIGatewayWebSocketEvent with a context that always has a domainName and connectionID. We were using `!` for these fields before. Hopefully we can push this into `aws-lambda` and use use their events. TODO: - Doc updates in the readme if this approach is sound - Decide if we should put https into the generated endpoint domain
19c8560
to
1ed07a4
Compare
attempted a rebase |
39beefa
to
26e3cfc
Compare
26e3cfc
to
4806f8a
Compare
I've successfully finished rebasing and extending to the new ping/pong functionality. I'll give it some manual testing on my project |
This changes the input objects to the handler function so it doesn't match |
I've been meaning to come back to this - plan to give it another pass over the next week |
In production I lightened up the types to match only the apis and arguments we use to allow for adapters. |
Say you want to use a custom endpoint for your management api communication. Say you want specific configuration? Well now you can! By providing an already constructed management api class. If you don't want to, no problem, we'll assume your endpoint for you from the incoming websocket event.
!
for these fields before. Hopefully we can push this intoaws-lambda
and use use their events.TODO:
closes #8