fix: improve kafka bindings #678
Conversation
oviecodes
requested review from
fmvilas,
Souvikns,
KhudaDad414 and
asyncapi-bot-eve
as code owners
src/adapters/kafka/index.ts
Outdated
| ) | ||
| const userAndPasswordSecurityReq = securityRequirements.find( | ||
| (sec) => sec.type() === 'userPassword' | ||
| (sec) => sec.type === 'userPassword' |
There was a problem hiding this comment.
@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 🤔
There was a problem hiding this comment.
alright @KhudaDad414 .
|
@oviecodes the branch has some conflicts. |
oviecodes
force-pushed
the
improve_kafka_bindings
branch
from
b9f9d84
to
1d2a50f
Compare
|
Pull Request Test Coverage Report for Build 9096781342Details
💛 - Coveralls |
|
@oviecodes you have a lot of conflicts, do you need any help? |
|
I'll fix it @Souvikns , I've been a little occupied lately. |
|
@oviecodes This PR needs a little love, if you have the time of course. |
|
I'll fix this @KhudaDad414 ... |
|
|
@KhudaDad414 could you check this out, so I can continue with the main issue |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ...
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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. 😆
|
@KhudaDad414 I'll open another pull request, I'll close this one |
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
setUpReplyMiddlewaresin thesrc/index.jsfile.Related issue(s)
Fixes #673