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 AMQP 0-9-1 support #456

Closed
wants to merge 16 commits into from

Conversation

AceTheCreator
Copy link
Member

@AceTheCreator AceTheCreator commented Jun 1, 2023

This PR adds support for AMQP 0-9-1.

cc @fmvilas @Souvikns @KhudaDad414

fixes: #257

@KhudaDad414 KhudaDad414 changed the title feat: added AMQP support feat: add AMQP 0-9-1 support Jun 12, 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.

Left a few comments.

Comment on lines 36 to 37
keepalive: serverBindings?.keepAlive || 0,
vhost: serverBindings?.vhost || '/',
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we have these properties in server binding(or any server bindings for that matter). 🤔
https://github.com/asyncapi/bindings/tree/master/amqp

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, we don't. I was playing around trying a default connection. I'll definitely get rid of it



async _connect(): Promise<this> {
const resolved = false
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the point of this variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right!

src/adapters/amqp/index.ts Show resolved Hide resolved
vhost: serverBindings?.vhost || '/',
heartbeat: serverBindings?.heartbeat || 0,
protocolVersion,
} as any)
Copy link
Member

Choose a reason for hiding this comment

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

any reason we supressing typescript typechecking here?

@sonarcloud
Copy link

sonarcloud bot commented Jun 20, 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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5326088969

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 60.53%

Totals Coverage Status
Change from base Build 5142697538: 0.0%
Covered Lines: 324
Relevant Lines: 454

💛 - Coveralls

@AceTheCreator
Copy link
Member Author

@KhudaDad414 i updated this PR, have a look

@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 ❤️

@KhudaDad414
Copy link
Member

this issue will be included as a feature after we release the v1 of glee. supporting a new protocol isn't the priority for now.

Copy link
Contributor

github-actions bot commented Mar 2, 2024

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 Mar 2, 2024
@github-actions github-actions bot closed this Jun 30, 2024
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.

Add support for AMQP 0-9-1
3 participants