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 WebSocket clients #267

Closed

Conversation

deepanshu44
Copy link

resolves issue #259

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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 8, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
90.3% 90.3% Duplication

@deepanshu44
Copy link
Author

@fmvilas I also have a question,
I cannot understand when the connection to remote is established.
I mean Glee lib has a connect method, but at what point it is executed and when?

@Souvikns
Copy link
Member

Souvikns commented Mar 8, 2022

I think when you emit the connect event in the adapter

this.emit('connect', { name: this.name(), adapter: this, connection: ws, channel: pathname })
is when the connect function in glee.ts invoked.
async connect(): Promise<any[]> {

I am new to the project so I might be wrong

@fmvilas
Copy link
Member

fmvilas commented Mar 9, 2022

It's called here: https://github.com/asyncapi/glee/blob/master/src/index.ts#L169. Notice the connect method on Glee has a listen alias. This lib used to be publicly exposed, therefore the alias.

@derberg
Copy link
Member

derberg commented Apr 4, 2022

@deepanshu44 do you plan to continue working on it? Open Force ended but since you started during the event, it is ok if you complete work later

@deepanshu44
Copy link
Author

@derberg I am really sorry about this. I will get this done in this week.
I am really sorry

@derberg
Copy link
Member

derberg commented Apr 5, 2022

@deepanshu44 no need to be sorry. I'm just making sure all is fine. I know many of our contributors, especially those coming from initiatives like Open Force, are mostly students and have their studies-related duties. Cheers 🍺

_connect(): Promise<this> {
return new Promise((resolve, reject) => {
const serverUrl = new URL(this.serverUrlExpanded)
const wsHttpServer = this.glee.options?.websocket?.httpServer || http.createServer()
Copy link
Member

Choose a reason for hiding this comment

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

Since we are creating ws client I think we should do something like this

Suggested change
const wsHttpServer = this.glee.options?.websocket?.httpServer || http.createServer()
const wsClient = new Websocket(serverUrl);

Copy link
Member

Choose a reason for hiding this comment

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

This is the WS server but you're right, we should be doing this on the client, which is in the ws-client-/indext.ts file.

Copy link
Member

@Souvikns Souvikns Apr 28, 2022

Choose a reason for hiding this comment

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

Actually, I was not able to comment on the ws-client/index.ts file because it was a file rename. Sorry for any confusion.

@deepanshu44
Copy link
Author

@fmvilas I have a very small doubt, why we want to create Websocket-client, as, as far as I know, we can create Websocket-client using asyncapi generator?

@fmvilas
Copy link
Member

fmvilas commented May 30, 2022

Generator and Glee serve two different purposes:

  1. Generator takes an AsyncAPI file and generates code from it. Glee doesn't generate anything. Glee is a client or server that is aware of the AsyncAPI file.
  2. Generator is executed only during development or CI. Glee is executed during development and production. It's a framework that uses AsyncAPI as a config file.
  3. With Generator's generated code, if someone touches the AsyncAPI file but doesn't execute Generator again, the code gets outdated and what's described in the AsyncAPI file might not match the code. The opposite is also very common. People update the code but don't update the AsyncAPI file, making the AsyncAPI file outdated. With Glee, this is impossible. If something is not defined in the AsyncAPI file, it can't be used in the code. If something is in the code but not in the AsyncAPI file, Glee will complain that it's not defined in the AsyncAPI file. That makes the code and the AsyncAPI file stay in sync forever. Pretty much like GraphQL does with the code and the schema.graphql file.

Does that make sense?

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 28, 2022
@fmvilas
Copy link
Member

fmvilas commented Sep 28, 2022

Closing this one in favor of #319.

@fmvilas fmvilas closed this Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants