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.

Sign up for GitHub

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

[v2] removed deprecated brokerList #882

Merged
merged 3 commits into from
Jun 17, 2020
Merged

[v2] removed deprecated brokerList #882

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba375ee
removed deprecated brokerList
samuelmacko Jun 16, 2020
a3d897c
change in CHANGELOG
samuelmacko Jun 17, 2020
0d012ce
change in CHANGELOG
samuelmacko Jun 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

## Deprecations

- As of v1.3, support for `brokerList` is deprecated for our Kafka topic scaler and will be removed in v2.0 ([#632](https://github.com/kedacore/keda/issues/632))

## History

- [v2.0.0](#v200)
Expand Down Expand Up @@ -81,7 +79,7 @@ None.
### Improvements

- Make targetQueryValue configurable in postgreSQL scaler ([#643](https://github.com/kedacore/keda/pull/643))
- Added bootstrapServers to deprecate brokerList ([#621](https://github.com/kedacore/keda/pull/621))
- Removed deprecated brokerList ([#621](https://github.com/kedacore/keda/pull/621))
Copy link
Member

Choose a reason for hiding this comment

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

This should be in section ## v2.0.0

Copy link
Member

Choose a reason for hiding this comment

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

And it should point to this PR

- Removed the need for deploymentName label ([#644](https://github.com/kedacore/keda/pull/644))
- Adding Kubernetes recommended labels to resources ([#596](https://github.com/kedacore/keda/pull/596))

Expand Down
12 changes: 2 additions & 10 deletions pkg/scalers/kafka_scaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,12 @@ func NewKafkaScaler(resolvedEnv, metadata, authParams map[string]string) (Scaler
func parseKafkaMetadata(resolvedEnv, metadata, authParams map[string]string) (kafkaMetadata, error) {
meta := kafkaMetadata{}

// brokerList marked as deprecated, bootstrapServers is the new one to use
if metadata["brokerList"] != "" && metadata["bootstrapServers"] != "" {
return meta, errors.New("cannot specify both bootstrapServers and brokerList (deprecated)")
}
if metadata["brokerList"] == "" && metadata["bootstrapServers"] == "" {
return meta, errors.New("no bootstrapServers or brokerList (deprecated) given")
if metadata["bootstrapServers"] == "" {
return meta, errors.New("no bootstrapServers given")
}
if metadata["bootstrapServers"] != "" {
meta.bootstrapServers = strings.Split(metadata["bootstrapServers"], ",")
}
if metadata["brokerList"] != "" {
kafkaLog.V(0).Info("WARNING: usage of brokerList is deprecated. use bootstrapServers instead.")
meta.bootstrapServers = strings.Split(metadata["brokerList"], ",")
}

if metadata["consumerGroup"] == "" {
return meta, errors.New("no consumer group given")
Expand Down
21 changes: 4 additions & 17 deletions pkg/scalers/kafka_scaler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ type parseKafkaMetadataTestData struct {

// A complete valid metadata example for reference
var validMetadata = map[string]string{
"brokerList": "broker1:9092,broker2:9092",
"consumerGroup": "my-group",
"topic": "my-topic",
"bootstrapServers": "broker1:9092,broker2:9092",
"consumerGroup": "my-group",
"topic": "my-topic",
}

// A complete valid authParams example for sasl, with username and passwd
Expand All @@ -32,22 +32,9 @@ var validWithAuthParams = map[string]string{
var validWithoutAuthParams = map[string]string{}

var parseKafkaMetadataTestDataset = []parseKafkaMetadataTestData{
// failure, no brokerList (deprecated) or bootstrapServers
// failure, no bootstrapServers
{map[string]string{}, true, 0, nil, "", ""},
// failure, both brokerList (deprecated) and bootstrapServers
{map[string]string{"brokerList": "foobar:9092", "bootstrapServers": "foobar:9092"}, true, 0, nil, "", ""},

// tests with brokerList (deprecated)
// failure, no consumer group
{map[string]string{"brokerList": "foobar:9092"}, true, 1, []string{"foobar:9092"}, "", ""},
// failure, no topic
{map[string]string{"brokerList": "foobar:9092", "consumerGroup": "my-group"}, true, 1, []string{"foobar:9092"}, "my-group", ""},
// success
{map[string]string{"brokerList": "foobar:9092", "consumerGroup": "my-group", "topic": "my-topic"}, false, 1, []string{"foobar:9092"}, "my-group", "my-topic"},
// success, more brokers
{map[string]string{"brokerList": "foo:9092,bar:9092", "consumerGroup": "my-group", "topic": "my-topic"}, false, 2, []string{"foo:9092", "bar:9092"}, "my-group", "my-topic"},

// tests with bootstrapServers
// failure, no consumer group
{map[string]string{"bootstrapServers": "foobar:9092"}, true, 1, []string{"foobar:9092"}, "", ""},
// failure, no topic
Expand Down