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

feat: Add support for non-serverless skill invocation #666

Closed
wants to merge 2 commits into from

Conversation

vmwxiong
Copy link

@vmwxiong vmwxiong commented Nov 20, 2020

Description

Kind of a wonky hack, but essentially allows a skill hosted on a server to use the websocket service and intercept skill requests to a local development machine.

Probably not going to update this PR since I've got what I need working for my dev workflow, but feel free to run with this idea if y'all feel like it's worth supporting.

Motivation and Context

Wanted to do some local development without needing to spin up ngrok and point the skill's endpoint to my ngrok URL all the time.

Testing

Iterating on a skill with this setup

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs(Add new document content)
  • Translate Docs(Translate document content)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • My commit message follows Conventional Commit Guideline

License

  • I confirm that this pull request can be released under the Apache 2 license

@Shreyas-vgr Shreyas-vgr requested a review from a team February 3, 2021 19:24
@sattpat
Copy link
Contributor

sattpat commented Feb 4, 2021

Thanks for the PR @vmwxiong . I just want to understand your usecase little bit. So the reason the ask-sdk-local-debug component was developed was to enable skill developers to run their skill code on their dev machine serverlessly. In your case you are routing the skill traffic to your Http server? My question is why arent you directly using the http endpoint? Do you not want to restrict yourself to a static endpoint?

@vmwxiong
Copy link
Author

vmwxiong commented Feb 5, 2021

Yup, that's exactly right. Basically we have a static endpoint as a shared environment, and each individual developer does development and testing on their local machine.

@sattpat
Copy link
Contributor

sattpat commented Feb 5, 2021

So why do you need this remote url setup? For the duration of a debugging session, the requests made by a developer for a skill get routed to their dev machine. But requests for all other devs will get routed using the static endpoint. So a debugging session is scoped to one developer account. Once the dev stops debugging, they go back to using the static endpoint. I am still not clear, why you need to forward the requests to the static endpoint? Sorry If I am missing something here.

@vmwxiong
Copy link
Author

vmwxiong commented Feb 5, 2021

You have all of that correct. Essentially, this is what allows us to parallelize development using server based architecture. If 5 devs are simultaneously working on the same skill, they all need to be able to run the server on their local machine and test against it, without interfering with one another.

The shared static endpoint is primarily used for QA, or sharing versions of the skill with non-developers.

@sattpat
Copy link
Contributor

sattpat commented Feb 5, 2021

Interesting. But I am going to hold off on the PR for now. Since the primary use case we are driving towards is devs running their code 'serverlessly'. Thanks for bringing up this usecase though. 😄

@sattpat sattpat closed this Feb 5, 2021
@vmwxiong
Copy link
Author

vmwxiong commented Feb 5, 2021

Sure thing, hope y'all keep us server-based folk in mind though, at least in terms of breaking changes 👍

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

2 participants