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

Add prefix for topics #5062

Merged
merged 1 commit into from
May 6, 2021
Merged

Conversation

ningyougang
Copy link
Contributor

@ningyougang ningyougang commented Feb 5, 2021

Description

This pr's target is, add prefix for all topics and add extra prefix for userEvent topic only.
After applied this pr, some zones(e.g. test1, test2, test3) can share same kafka instance using different prefix topic,
BTW, userEvent is special, user may want to send some zones's event to same topic, so added extra prefix for it, user can decide whether want to send the event to same topic or different topic.

I tested in my local env

  • Default configuration (all topics's prefix is empty string), topics name as below
    image

  • kafka_topics_prefix: "blue_" and kafka_topics_userEvent_prefix: "test_"
    image

And the activations are successful on above 2 cases both.

Related issue and scope

  • I opened an issue to propose and discuss this change (#????)

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@ningyougang ningyougang closed this Feb 7, 2021
@ningyougang ningyougang reopened this Feb 7, 2021
@ningyougang ningyougang closed this Feb 8, 2021
@ningyougang ningyougang reopened this Feb 8, 2021
@codecov-io
Copy link

Codecov Report

Merging #5062 (eb2e900) into master (1753946) will decrease coverage by 6.92%.
The diff coverage is 53.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5062      +/-   ##
==========================================
- Coverage   82.80%   75.88%   -6.93%     
==========================================
  Files         207      207              
  Lines       10034    10047      +13     
  Branches      444      418      -26     
==========================================
- Hits         8309     7624     -685     
- Misses       1725     2423     +698     
Impacted Files Coverage Δ
...rg/apache/openwhisk/core/scheduler/Scheduler.scala 0.00% <0.00%> (ø)
.../apache/openwhisk/core/controller/Controller.scala 55.66% <33.33%> (+0.85%) ⬆️
...scala/org/apache/openwhisk/common/UserEvents.scala 75.00% <50.00%> (+8.33%) ⬆️
.../scala/org/apache/openwhisk/core/WhiskConfig.scala 95.36% <100.00%> (+0.06%) ⬆️
...apache/openwhisk/core/ack/MessagingActiveAck.scala 58.33% <100.00%> (+3.78%) ⬆️
...nwhisk/core/database/RemoteCacheInvalidation.scala 80.64% <100.00%> (+0.64%) ⬆️
...enwhisk/core/loadBalancer/CommonLoadBalancer.scala 72.56% <100.00%> (ø)
...che/openwhisk/core/loadBalancer/LoadBalancer.scala 76.92% <100.00%> (ø)
...e/loadBalancer/ShardingContainerPoolBalancer.scala 80.79% <100.00%> (ø)
...la/org/apache/openwhisk/core/invoker/Invoker.scala 70.31% <100.00%> (+0.47%) ⬆️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1753946...eb2e900. Read the comment docs.

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM

@bdoyle0182
Copy link
Contributor

LGTM

- Add prefix for topics
- Add extra prefix for userEvent topic only
@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2021

Codecov Report

Merging #5062 (1b73098) into master (f7ec9e3) will increase coverage by 35.84%.
The diff coverage is 53.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #5062       +/-   ##
===========================================
+ Coverage   38.79%   74.63%   +35.84%     
===========================================
  Files         218      220        +2     
  Lines       10700    10822      +122     
  Branches      454      445        -9     
===========================================
+ Hits         4151     8077     +3926     
+ Misses       6549     2745     -3804     
Impacted Files Coverage Δ
...rg/apache/openwhisk/core/scheduler/Scheduler.scala 0.00% <0.00%> (ø)
.../apache/openwhisk/core/controller/Controller.scala 55.66% <33.33%> (+55.66%) ⬆️
...scala/org/apache/openwhisk/common/UserEvents.scala 75.00% <50.00%> (+8.33%) ⬆️
...la/org/apache/openwhisk/core/invoker/Invoker.scala 46.93% <66.66%> (-22.91%) ⬇️
.../scala/org/apache/openwhisk/core/WhiskConfig.scala 95.54% <100.00%> (+0.05%) ⬆️
...apache/openwhisk/core/ack/MessagingActiveAck.scala 58.33% <100.00%> (+3.78%) ⬆️
...nwhisk/core/database/RemoteCacheInvalidation.scala 80.64% <100.00%> (+0.64%) ⬆️
...enwhisk/core/loadBalancer/CommonLoadBalancer.scala 72.56% <100.00%> (+72.56%) ⬆️
...che/openwhisk/core/loadBalancer/LoadBalancer.scala 76.92% <100.00%> (+76.92%) ⬆️
...e/loadBalancer/ShardingContainerPoolBalancer.scala 80.79% <100.00%> (+80.79%) ⬆️
... and 136 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7ec9e3...1b73098. Read the comment docs.

@style95 style95 merged commit aa7e6e2 into apache:master May 6, 2021
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.

6 participants