Skip to content

refactor pulsar scaler#6848

Merged
JorTurFer merged 3 commits into
kedacore:mainfrom
rickbrouwer:pulsar
Aug 7, 2025
Merged

refactor pulsar scaler#6848
JorTurFer merged 3 commits into
kedacore:mainfrom
rickbrouwer:pulsar

Conversation

@rickbrouwer

@rickbrouwer rickbrouwer commented Jun 18, 2025

Copy link
Copy Markdown
Member

Refactor pulsar scaler

Checklist

Relates to #5797

@rickbrouwer
refactor pulsar scaler
d4a2a48 
Signed-off-by: Rick Brouwer <[email protected]>
@rickbrouwer rickbrouwer requested a review from a team as a code owner June 18, 2025 19:11
@rickbrouwer

rickbrouwer commented Jun 18, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e pulsar
Update: You can check the progress here

dttung2905
dttung2905 reviewed Jun 18, 2025
View reviewed changes

@dttung2905 dttung2905 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally LGTM, just 1 tiny nit from me and we are good to go

Comment thread pkg/scalers/pulsar_scaler.go Outdated
@rickbrouwer

rickbrouwer commented Jun 19, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e pulsar
Update: You can check the progress here

@rickbrouwer
Update after feedback
bf3619d 
Signed-off-by: Rick Brouwer <[email protected]>
@rickbrouwer

rickbrouwer commented Jun 19, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e pulsar
Update: You can check the progress here

@dttung2905 dttung2905 mentioned this pull request Jun 30, 2025
Closed
58 tasks
@rickbrouwer rickbrouwer mentioned this pull request Jul 1, 2025
Merged
7 tasks
@rickbrouwer

rickbrouwer commented Jul 23, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e pulsar
Update: You can check the progress here

@rickbrouwer rickbrouwer added the ok-to-merge This PR can be merged label Jul 24, 2025
@rickbrouwer

rickbrouwer commented Jul 24, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e pulsar
Update: You can check the progress here

@JorTurFer JorTurFer merged commit 48db78a into kedacore:main Aug 7, 2025
24 checks passed
wozniakjan
wozniakjan reviewed Aug 7, 2025
View reviewed changes
Comment thread pkg/scalers/pulsar_scaler.go
Comment on lines +47 to +52
OauthTokenURI string `keda:"name=oauthTokenURI, order=triggerMetadata, optional"`
Scope string `keda:"name=scope, order=triggerMetadata, optional"`
ClientID string `keda:"name=clientID, order=triggerMetadata, optional"`
EndpointParams string `keda:"name=EndpointParams, order=triggerMetadata, optional"`

pulsarAuth *authentication.AuthMeta

@wozniakjan wozniakjan Aug 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

any chance this can be replaced by authentication.Config? No problem if not, just thought it might simplify the code a bit if it helps.

keda/pkg/scalers/authentication/authentication_types.go

Lines 95 to 104 in 4b5c62e

// Config is the configuration for the authentication types
type Config struct {
Modes []Type `keda:"name=authModes, order=triggerMetadata, enum=apiKey;basic;tls;bearer;custom;oauth, exclusiveSet=bearer;basic;oauth, optional"`
BearerToken string `keda:"name=bearerToken, order=authParams, optional"`
BasicAuth `keda:"optional"`
CertAuth `keda:"optional"`
OAuth `keda:"optional"`
CustomAuth `keda:"optional"`
}

@rickbrouwer rickbrouwer Aug 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, it just happened to be merged when you posted your comment. Perhaps consider converting this request to an issue with a "good-first-issue" label?

@wozniakjan wozniakjan Aug 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah, race condition with maintainer executions, happens all the time...

@JorTurFer JorTurFer Aug 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is my fault, I shouldn't review and merge anything else 😄 Just live and enjoy 🤣
Yeah, let's open an issue for this, it's a quite good good-first-issue

@wozniakjan wozniakjan Aug 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

all good, technically I reviewed late according to the timestamps

@rickbrouwer rickbrouwer deleted the pulsar branch August 7, 2025 12:40
@wozniakjan wozniakjan mentioned this pull request Aug 8, 2025
Merged
3 tasks
dpochopsky pushed a commit to dpochopsky/keda that referenced this pull request Sep 12, 2025
@rickbrouwer @dpochopsky
refactor pulsar scaler (kedacore#6848)
334d4a0 
* refactor pulsar scaler

Signed-off-by: Rick Brouwer <[email protected]>

* Update after feedback

Signed-off-by: Rick Brouwer <[email protected]>

---------

Signed-off-by: Rick Brouwer <[email protected]>
Signed-off-by: David Pochopsky <[email protected]>
alt-dima pushed a commit to alt-dima/keda that referenced this pull request Dec 13, 2025
@rickbrouwer @alt-dima
refactor pulsar scaler (kedacore#6848)
f90e187 
* refactor pulsar scaler

Signed-off-by: Rick Brouwer <[email protected]>

* Update after feedback

Signed-off-by: Rick Brouwer <[email protected]>

---------

Signed-off-by: Rick Brouwer <[email protected]>
Signed-off-by: Dmitriy Altuhov <[email protected]>
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Dec 22, 2025
@rickbrouwer
refactor pulsar scaler (kedacore#6848)
e7109ed 
* refactor pulsar scaler

Signed-off-by: Rick Brouwer <[email protected]>

* Update after feedback

Signed-off-by: Rick Brouwer <[email protected]>

---------

Signed-off-by: Rick Brouwer <[email protected]>
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Feb 13, 2026
@rickbrouwer @tangobango5
refactor pulsar scaler (kedacore#6848)
6ea7b9c 
* refactor pulsar scaler

Signed-off-by: Rick Brouwer <[email protected]>

* Update after feedback

Signed-off-by: Rick Brouwer <[email protected]>

---------

Signed-off-by: Rick Brouwer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wozniakjan wozniakjan wozniakjan left review comments
@JorTurFer JorTurFer JorTurFer approved these changes
@zroubalik zroubalik zroubalik approved these changes
+1 more reviewer
@dttung2905 dttung2905 dttung2905 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ok-to-merge This PR can be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rickbrouwer @zroubalik @wozniakjan @dttung2905 @JorTurFer