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 cassandraCreateSchema affinity #1475

Conversation

chasekiefer
Copy link
Contributor

Signed-off-by: Chase Kiefer [email protected]

Resolves #1471

Short description of the changes

  • add affinity to cassandraCreateSchema type
  • Modified cassandraDeps for cassandraCreateSchema affinity
  • add unit test TestCassandraCreateSchemaAffinity

@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #1475 (770943a) into master (c49f0cd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 770943a differs from pull request most recent head d4ff6e0. Consider uploading reports for the commit d4ff6e0 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1475   +/-   ##
=======================================
  Coverage   87.21%   87.21%           
=======================================
  Files          90       90           
  Lines        5004     5005    +1     
=======================================
+ Hits         4364     4365    +1     
  Misses        484      484           
  Partials      156      156           
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100.00% <ø> (ø)
pkg/storage/cassandra_dependencies.go 100.00% <100.00%> (ø)

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 c49f0cd...d4ff6e0. Read the comment docs.

Copy link

@LuChenjing LuChenjing left a comment

Choose a reason for hiding this comment

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

Looks great, wish to been merged

Copy link
Collaborator

@rubenvp8510 rubenvp8510 left a comment

Choose a reason for hiding this comment

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

LGTM

@jpkrohling
Copy link
Contributor

This needs to be rebased.

@chasekiefer chasekiefer force-pushed the 1471-cassandraCreateSchema-node-affinity branch from faa763d to 7133f82 Compare June 24, 2021 21:19
@jpkrohling
Copy link
Contributor

End-to-end tests restarted.

@jpkrohling
Copy link
Contributor

The tests keep failing: were you able to run them locally? Have you tried this change in a minikube cluster?

@rubenvp8510
Copy link
Collaborator

I don't think the e2e failing is related to the changes made in this PR.

There is an issue with the ingress test, which should be fixed by this PR: #1491 As soon as that PR is merged could you rebase?

@chasekiefer
Copy link
Contributor Author

@rubenvp8510 Yes, I can. Thank you for pointing this out.

@jpkrohling
Copy link
Contributor

That PR was merged and I just rebased this PR.

@jpkrohling jpkrohling enabled auto-merge (squash) June 28, 2021 15:09
@jpkrohling jpkrohling merged commit bd71dfd into jaegertracing:master Jul 2, 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.

cassandraCreateSchema job not support specify node to schedule
4 participants