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

Jump to bottom

Group Websocket-related properties under provider.websocket #10960

Open
1 task done
pgrzesik opened this issue Apr 11, 2022 · 10 comments · Fixed by #11347
Open
1 task done

Group Websocket-related properties under provider.websocket #10960

pgrzesik opened this issue Apr 11, 2022 · 10 comments · Fixed by #11347
Labels
deprecation Deprecation proposal (breaking with next major) good first issue help wanted refactor

Comments

@pgrzesik
Copy link
Contributor

pgrzesik commented Apr 11, 2022

Is there an existing issue for this?

  • I have searched existing issues, it hasn't been reported yet

Use case description

Currently, we have websocket-related properties configured directly on provider level:

  • provider.websocketsApiName
  • provider.websocketsApiRouteSelectionExpression
  • provider.websocketsDescription

This is not consistent with how iam, httpApi or apiGateway is configured on provider level.

Proposed solution (optional)

In order to be consistent with how iam, httpApi or apiGateway is configured on provider level, it would be great to group all websockets-related properties under provider.websocket namespace.

As the change is breaking, we should add support for new properties and introduce deprecation for old ones, aiming for removal of support for old syntax in next major version.

Similar issues from the past to serve as inspiration:

PRs welcome :raised
@pgrzesik pgrzesik added good first issue refactor help wanted deprecation Deprecation proposal (breaking with next major) labels Apr 11, 2022
@pgrzesik pgrzesik mentioned this issue Apr 11, 2022
Open
@BillConley01
Copy link

BillConley01 commented Apr 11, 2022

Is something like this edit to the serverless.yml.md file what you are looking for?
websockets:
apiName: custom-websockets-api-name
# custom route selection expression
apiRouteSelectionExpression: $request.body.route
# Use a custom description for the websockets API
description: Custom Serverless Websockets

@pgrzesik
Copy link
Contributor Author

pgrzesik commented Apr 11, 2022

Hey @BillConley01 - it's a little bit more than that - please check the linked issues and related PRs to see how the scope of this looks like (it's very similar scope as one for the linked PRs)

@BillConley01
Copy link

BillConley01 commented Apr 12, 2022

I created a draft pull request for this. Could you please take a look. I am having problems with a test failing.

@pgrzesik
Copy link
Contributor Author

pgrzesik commented Apr 12, 2022

Hey @BillConley01 - I've reviewed the current version - let's continue our discussion in PR threads

@BillConley01
Copy link

BillConley01 commented Apr 12, 2022

sure

@dheeraj1997
Copy link

dheeraj1997 commented May 24, 2022

I would like to contribute here. Is this issue still open?

@pgrzesik
Copy link
Contributor Author

pgrzesik commented May 24, 2022

Hello @dheeraj1997 - @BillConley01 is currently working on it

@sahilsuman933
Copy link

sahilsuman933 commented Jul 27, 2022

@pgrzesik, can I work on this issue?

@pgrzesik
Copy link
Contributor Author

pgrzesik commented Jul 28, 2022

Sure @sahilsuman933, it would be awesome 🙌 It seems like @BillConley01 didn't have a chance to wrap this up

@sahilsuman933
Copy link

sahilsuman933 commented Aug 8, 2022

@pgrzesik I am not able to solve this issue. I am getting like 300+ errors, and I don't know why. Should I raise a PR, and can you help me there?

@puneetkathar1 puneetkathar1 mentioned this issue Aug 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
deprecation Deprecation proposal (breaking with next major) good first issue help wanted refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix-#10960 puneetkathar1/serverless
4 participants
@pgrzesik @dheeraj1997 @sahilsuman933 @BillConley01