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 http adapter support for glee #320

Merged
merged 56 commits into from
Mar 22, 2023

Conversation

ritik307
Copy link
Contributor

@ritik307 ritik307 commented Jun 26, 2022

Description

Implement the HTTP adapter for both server and client

Aims to -

  • connect HTTP server as per the specification.
  • Handle incoming messages only on the channels specified in the specification
  • Send appropriate responses from the registered function.
  • connect the HTTP client as per the specification.
  • HTTP client should be able to connect via any other protocol and provide response in the specified registered function.

Steps to test the codeflow

Steps to run the base code

  1. npm i
  2. npm run dev

Steps to run the server
4. cd examples/anime-http/server
5. npm i
6. npm run dev

Steps to run the client
8. cd examples/anime-http/client
9. npm i
10. npm run dev

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@ritik307 ritik307 changed the title adding http header init feat: adding http header init Jun 26, 2022
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Left a few comments.

Along with those, I'm adding two more generic comments here:

  1. What does "adding HTTP header init" means? 😄
  2. It would be great if you can consider making it clear when it should be creating an HTTP server and when it should be creating a client (client can be done in another PR). For instance, if the server is marked as "local", then it should create a server. Otherwise, it shouldn't. See an example here: https://github.com/asyncapi/glee/pull/319/files#diff-5a59976f2bbdbefa8fb1b10842622e9c61def3c47869b93b2d0d3d1374530734R21.

examples/simple/asyncapi.yaml Outdated Show resolved Hide resolved
examples/simple/.glee/functions/onTest.js Outdated Show resolved Hide resolved
examples/simple/asyncapi.yaml Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
src/adapters/http/index.ts Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jul 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ritik307
Copy link
Contributor Author

Left a few comments.

Along with those, I'm adding two more generic comments here:

  1. What does "adding HTTP header init" means? smile
  2. It would be great if you can consider making it clear when it should be creating an HTTP server and when it should be creating a client (client can be done in another PR). For instance, if the server is marked as "local", then it should create a server. Otherwise, it shouldn't. See an example here: https://github.com/asyncapi/glee/pull/319/files#diff-5a59976f2bbdbefa8fb1b10842622e9c61def3c47869b93b2d0d3d1374530734R21.

Sorry for that I will try to be more clear on the commit messages 🙂

@ritik307
Copy link
Contributor Author

@fmvilas Can you kindly provide me with some resources to study web sockets, I am baffled with web sockets at the moment.😟

@fmvilas
Copy link
Member

fmvilas commented Jul 11, 2022

Sure, MDN has a great resource: https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API.

src/adapters/http/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

Since you are adding a new adapter to support the HTTP server you have to update the docs accordingly.

.gitignore Outdated Show resolved Hide resolved
examples/simple/.glee/functions/onTest.js Outdated Show resolved Hide resolved
examples/simple/asyncapi.yaml Outdated Show resolved Hide resolved
@ritik307
Copy link
Contributor Author

@Souvikns I have changed the examples/simple/asyncapi.yaml file according to the docs that you have referred but i am not sure whether i used $ref: '#/components/messages/testReply' at correct place or not. Kindly check it and let me know if any further changes are required

@sonarcloud
Copy link

sonarcloud bot commented Dec 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ritik307 ritik307 requested review from Souvikns and KhudaDad414 and removed request for Souvikns March 7, 2023 09:35
@ritik307 ritik307 requested review from fmvilas and removed request for KhudaDad414 March 8, 2023 11:06
KhudaDad414
KhudaDad414 previously approved these changes Mar 9, 2023
Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Left some comments

src/lib/message.ts Outdated Show resolved Hide resolved
src/lib/message.ts Outdated Show resolved Hide resolved
@ritik307
Copy link
Contributor Author

ritik307 commented Mar 9, 2023

@fmvilas Made the changes you have suggested. Please check

@ritik307 ritik307 requested a review from fmvilas March 9, 2023 17:49
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Left some more comments. Please don't rush it out. Take your time to evaluate what's the best solution. For instance, I see you're repeating { [key: string]: string } | { [key: string]: string[] } in multiple places. Maybe it's time to define it as a custom type to reuse it.

src/lib/message.ts Outdated Show resolved Hide resolved
src/lib/message.ts Outdated Show resolved Hide resolved
@ritik307 ritik307 requested a review from fmvilas March 10, 2023 04:29
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Left a few more suggestions

src/adapters/http/client.ts Outdated Show resolved Hide resolved
src/adapters/http/server.ts Outdated Show resolved Hide resolved
src/adapters/http/server.ts Outdated Show resolved Hide resolved
src/lib/functions.ts Show resolved Hide resolved
src/lib/index.d.ts Outdated Show resolved Hide resolved
src/lib/message.ts Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Mar 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ritik307 ritik307 requested a review from fmvilas March 11, 2023 10:06
Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Souvikns
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 2557652 into asyncapi:master Mar 22, 2023
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

SinghiHarsh pushed a commit to SinghiHarsh/glee that referenced this pull request Mar 27, 2023
Co-authored-by: Ritik Rawal <ritikrawal@MacBook-Pro-5.local>%0ACo-authored-by: Ritik Rawal <ritikrawal@MacBook-Pro-85.local>%0ACo-authored-by: Ritik Rawal <ritikrawal@ip-192-168-0-101.ap-south-1.compute.internal>%0ACo-authored-by: Ritik Rawal <ritikrawal@ip-192-168-0-102.ap-south-1.compute.internal>
SinghiHarsh pushed a commit to SinghiHarsh/glee that referenced this pull request Mar 28, 2023
Co-authored-by: Ritik Rawal <ritikrawal@MacBook-Pro-5.local>%0ACo-authored-by: Ritik Rawal <ritikrawal@MacBook-Pro-85.local>%0ACo-authored-by: Ritik Rawal <ritikrawal@ip-192-168-0-101.ap-south-1.compute.internal>%0ACo-authored-by: Ritik Rawal <ritikrawal@ip-192-168-0-102.ap-south-1.compute.internal>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants