-
Notifications
You must be signed in to change notification settings - Fork 4
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 nodes validation in farmerbot #1135
Merged
Eslam-Nawara
merged 17 commits into
development
from
development_farmerbot_input_validation
Aug 4, 2024
Merged
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
33ce72d
feat: add nodes validation
SalmaElsoly 1cc3691
fix: GetNodes mock calling times
SalmaElsoly 7ac19aa
fix: correct substrate to be sended to validateInput
SalmaElsoly 66d3420
refactor: use contains instead of index + separate code in another file
SalmaElsoly 6fc7f59
refactor: send config by value
SalmaElsoly ab2feef
refactor: send config by value
SalmaElsoly db75d1b
refactor: wrapper for validateInput to create manager in it
SalmaElsoly 288133d
feat: add validation to never shutdown nodes + handle all nodes are i…
SalmaElsoly e36abda
refactor: split logic into other two function + used TD tests
SalmaElsoly f0eca89
refactor: seprate to validate each type in separate function
SalmaElsoly 95b9648
refactor: change error string
SalmaElsoly 7684830
refactor: change error string
SalmaElsoly 9ead0c5
refactor: remove excluded nodes from check
SalmaElsoly 01540e0
refactor: remove excluded nodes from check
SalmaElsoly ddd6533
refactor: remove unnecessary else nesting
SalmaElsoly 9a331f0
refactor: remove extra testing
SalmaElsoly 80e0e44
refactor: change variables name
SalmaElsoly File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,3 +94,41 @@ func ParseEnv(content string) (network string, mnemonicOrSeed string, keyType st | |
|
||
return | ||
} | ||
|
||
// ValidateInput validates that included, excluded and priority nodes are in the farm | ||
Eslam-Nawara marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func ValidateInput(input *internal.Config, sub internal.Substrate) error { | ||
nodes, err := sub.GetNodes(input.FarmID) | ||
if err != nil { | ||
return fmt.Errorf("couldn't retrieve node for %d : %v", input.FarmID, err) | ||
} | ||
nodesMap := make(map[uint32]bool) | ||
for _, node := range nodes { | ||
nodesMap[node] = true | ||
} | ||
|
||
//validate included nodes | ||
for _, includedNode := range input.IncludedNodes { | ||
if _, ok := nodesMap[includedNode]; !ok { | ||
return fmt.Errorf("included node with id %d doesn't exist in the farm", includedNode) | ||
} | ||
} | ||
//validate excluded nodes | ||
for _, excludedNode := range input.ExcludedNodes { | ||
if _, ok := nodesMap[excludedNode]; !ok { | ||
return fmt.Errorf("excluded node with id %d doesn't exist in the farm", excludedNode) | ||
} | ||
index := slices.Index(input.IncludedNodes, excludedNode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use |
||
if index >= 0 { | ||
return fmt.Errorf("cannot include and exclude the same node %d", excludedNode) | ||
} | ||
} | ||
|
||
//validate priority nodes | ||
for _, priorityNode := range input.PriorityNodes { | ||
index := slices.Index(input.IncludedNodes, priorityNode) | ||
if index < 0 { | ||
return fmt.Errorf("priority node with id %d doesn't exist included nodes", priorityNode) | ||
} | ||
} | ||
return nil | ||
} |
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
Oops, something went wrong.
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.
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.
no connection was opened here so no need to close the manager
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.
Substrate( ) by default returns a connection :
doesn't it make connection to db?
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.
ahaaa, that's kinda confusing, please create a manager then a connection, or at least rename to subConn, I couldn't see your're using a connection