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

fix: improve kafka bindings #678

Closed
wants to merge 21 commits into from

Conversation

oviecodes
Copy link
Contributor

Description

This PR contains a fix for the Kafka adapter. The Kafka adapter should now start properly and run well.

Kindly have a look @Souvikns @KhudaDad414 , we also need to discuss about the setUpReplyMiddlewares in the src/index.js file.

Related issue(s)
Fixes #673

@oviecodes oviecodes changed the title Improve kafka bindings Fix: Improve kafka bindings Jan 8, 2024
@oviecodes oviecodes changed the title Fix: Improve kafka bindings fix: Improve kafka bindings Jan 8, 2024
@oviecodes oviecodes changed the title fix: Improve kafka bindings fix: improve kafka bindings Jan 8, 2024
)
const userAndPasswordSecurityReq = securityRequirements.find(
(sec) => sec.type() === 'userPassword'
(sec) => sec.type === 'userPassword'
Copy link
Member

Choose a reason for hiding this comment

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

@oviecodes what do you think about extracting these constants inside an enum? then reference it here as something like SECURITY_TYPE.USER_PASSWORD or SECURITY_TYPE.SCRAM_SHA_512 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright @KhudaDad414 .

@KhudaDad414
Copy link
Member

@oviecodes the branch has some conflicts.

Copy link

sonarcloud bot commented Feb 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@coveralls
Copy link

coveralls commented Feb 15, 2024

Pull Request Test Coverage Report for Build 9096781342

Details

  • 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 72.714%

Totals Coverage Status
Change from base Build 8965267664: 0.0%
Covered Lines: 389
Relevant Lines: 486

💛 - Coveralls

@Souvikns
Copy link
Member

@oviecodes you have a lot of conflicts, do you need any help?

@oviecodes
Copy link
Contributor Author

oviecodes commented Mar 20, 2024

I'll fix it @Souvikns , I've been a little occupied lately.

@KhudaDad414
Copy link
Member

@oviecodes This PR needs a little love, if you have the time of course.

@oviecodes
Copy link
Contributor Author

oviecodes commented May 7, 2024

I'll fix this @KhudaDad414 ...

Copy link

sonarcloud bot commented May 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@oviecodes
Copy link
Contributor Author

@KhudaDad414 could you check this out, so I can continue with the main issue

Copy link
Member

Choose a reason for hiding this comment

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

Can you revert the changes to the docs? since this PR is about KAFKA bindings I don't think it's appropriate for it to include any other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this PR is old, I had to merge it with the current master branch, so there isn't any change that's not already on the master branch ...

Copy link
Member

@KhudaDad414 KhudaDad414 May 28, 2024

Choose a reason for hiding this comment

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

@oviecodes
If Diff shows there are, then there are changes that this PR is proposing.
for example. master doesn't have this file: docs/pages/glee-template.md since it was removed by a previous PR.
but you are adding it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh Damn, I'm just pulling from the master branch. I'll try updating from master again, if it doesn't work then I'll scrap this PR, it seems it's too old and things are getting a bit more complicated.

Copy link
Member

Choose a reason for hiding this comment

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

@oviecodes yep. I think the problem is that you are pulling from your fork's master instead of this repo's master. happened to me multiple times. 😆

@oviecodes oviecodes closed this Jul 9, 2024
@oviecodes
Copy link
Contributor Author

oviecodes commented Jul 9, 2024

@KhudaDad414 I'll open another pull request, I'll close this one

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.

5 participants