-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix work cache in cross-dc setup #442
Conversation
@@ -214,6 +214,38 @@ spec: | |||
EOF | |||
} | |||
|
|||
function deploy_replicated_cache_cr() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this effectively the same as deploy_distributed_cache_cr
except that we specify replicatedCache
? I think it would be simpler to just make the cache type a parameter of the original deploy_cache_cr
method so that we don't have to maintain duplicated code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryanemerson there is also the number of owners. I would say we can leave this as is now, as we plan to migrate to Helm anyway. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's fine, it was just a nitpick 👍
I can see the work cache stats coming up in a non cross-site deployment. LGTM. scratch my previous comment about the PR for cross-site deployment still in progress. I see thats merged yday. Will try to test this for a cross-site deployment. |
@ryanemerson / @mhajas / @kami619 - I saw the discussions above, and I wonder if this can be merged before today's meeting, and we can then plan the next steps? |
@ahus1 Sure, works for me. |
No description provided.