Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

add msk bridge and remove hardcoded kafka url #89

Merged
merged 2 commits into from
Sep 29, 2021
Merged

add msk bridge and remove hardcoded kafka url #89

merged 2 commits into from
Sep 29, 2021

Conversation

Zarrie
Copy link
Contributor

@Zarrie Zarrie commented Sep 21, 2021

@Zarrie Zarrie force-pushed the feature/UPPSF-2717-kafka-msk-bridge branch from 3aa4929 to 0a586dc Compare September 23, 2021 07:49
@coveralls
Copy link

coveralls commented Sep 23, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 35.385% when pulling 4f431cf on feature/UPPSF-2717-kafka-msk-bridge into 16fc50e on master.

@Zarrie Zarrie force-pushed the feature/UPPSF-2717-kafka-msk-bridge branch from 0a586dc to aff713a Compare September 23, 2021 10:10
@Zarrie Zarrie force-pushed the feature/UPPSF-2717-kafka-msk-bridge branch from aff713a to ca5ba50 Compare September 23, 2021 11:42
@Zarrie Zarrie marked this pull request as ready for review September 27, 2021 07:08
@Zarrie Zarrie requested a review from a team as a code owner September 27, 2021 07:08
@Zarrie Zarrie requested review from AlexKondov and a team September 27, 2021 07:08
@dtvalk-ov dtvalk-ov requested a review from a team September 27, 2021 08:29
atanasdinov
atanasdinov reviewed Sep 28, 2021
View reviewed changes
helm/coco-kafka-bridge/app-configs/kafka-bridge_eks_delivery_dev.yaml
replicas: 2
- name: msk-to-kafka-bridge
sourceKafkaProxyUrl: "http://kafka-rest-proxy-msk:8080"
groupIdPrefix: kafka-msk-bridge
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have unique consumer groups for these bridges?

atanasdinov
atanasdinov approved these changes Sep 28, 2021
View reviewed changes
Copy link
Contributor

@atanasdinov atanasdinov left a comment

Choose a reason for hiding this comment

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

LGTM. I'd only like to mention that a producer field needs to be added for the currently existing "proxy" bridges for the staging and prod environments.

@Zarrie
Copy link
Contributor Author

Zarrie commented Sep 28, 2021

LGTM. I'd only like to mention that a producer field needs to be added for the currently existing "proxy" bridges for the staging and prod environments.

It's intentionally not added, so we don't deploy it on staging/prod by mistake

@Zarrie Zarrie changed the base branch from master to kafka-dev September 29, 2021 07:56
@Zarrie Zarrie merged commit 59f0beb into kafka-dev Sep 29, 2021
@Zarrie Zarrie deleted the feature/UPPSF-2717-kafka-msk-bridge branch September 29, 2021 09:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MiroslavGatsanoga MiroslavGatsanoga MiroslavGatsanoga approved these changes

@atanasdinov atanasdinov atanasdinov approved these changes

@dtvalk-ov dtvalk-ov dtvalk-ov approved these changes

@AlexKondov AlexKondov Awaiting requested review from AlexKondov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Zarrie @coveralls @MiroslavGatsanoga @atanasdinov @dtvalk-ov