fix(torch): processing duplications #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jose Ramon Mañes <[email protected]>
smuu
reviewed
Nov 9, 2023
Bidon15
reviewed
Nov 9, 2023
Bidon15
suggested changes
Nov 9, 2023
…tate Signed-off-by: Jose Ramon Mañes <[email protected]>
Bidon15
suggested changes
Nov 10, 2023
pkg/k8s/statefulsets.go
Outdated
Comment on lines
47
to
52
| for event := range watcher.ResultChan() { | ||
| // Check if the event object is of type *v1.StatefulSet | ||
| if statefulSet, ok := event.Object.(*v1.StatefulSet); ok { | ||
| //log.Info("StatefulSet containers: ", statefulSet.Spec.Template.Spec.Containers) | ||
|
|
||
| // check if the node is DA, if so, send it to the queue to generate the multi address | ||
| if strings.HasPrefix(statefulSet.Name, "da") { | ||
| // Check if the StatefulSet is valid based on the conditions | ||
| if isStatefulSetValid(statefulSet) { | ||
| // Perform necessary actions, such as adding the node to the Redis queue |
There was a problem hiding this comment.
design wise, you can do the following:
- Check if not ok, fail fast with error and exit the for loop
- If ok, then you have a second if outside of the boundaries of the first if else one(where you have the ok checker)
- If you want to check all the objects in the channel, you need to store the error messages one by one and then exit the for loop. So far, as I understood the code, you are not intending to do so
There was a problem hiding this comment.
I've updated this part of the code, but the point 3, we will only receive one event per STS at a time, which means that in case this STS has any issues, we return the error immediately, wdyt?
Signed-off-by: Jose Ramon Mañes <[email protected]>
mojtaba-esk
reviewed
Nov 13, 2023
Signed-off-by: Jose Ramon Mañes <[email protected]>
Bidon15
approved these changes
Nov 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hello team!
small one, it fixes to add duplicates to the queue, that happened because the watcher sends an event for every container in the pod, but we only need it once.
cheers! 🚀
closes: https://github.com/celestiaorg/devops/issues/577
cc: @celestiaorg/devops