fix: multiaddress not constructed on startup #524
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
Restructure how multiaddresses and IP addresses are handled within the codebase to improve maintainability and efficiency. Added logic for obtaining GCP external IP addresses and restructured public address retrieval. Moved HTTP client functionality to a new package and updated references accordingly.
- Handle invalid multiaddresses gracefully - Continue searching for eligible workers on errors - Add more detailed logging for debugging - Prevent potential nil pointer dereference - Log warning if no workers found
|
PR description is too short and seems to not fulfill PR template, please fill in |
⚠ golint failed (.)Found 191 lint suggestions; failing. Show Detail |
⚠ gosec failed (.)Show Detail |
⚠ goimports failed (.)
|
⚠ shadow failed (.) |
⚠ errcheck failed (.) |
⚠ staticcheck failed (.) |
teslashibe
changed the title
teslashibe
changed the title
|
PR description is too short and seems to not fulfill PR template, please fill in |
1 similar comment
|
PR description is too short and seems to not fulfill PR template, please fill in |
This commit simplifies the worker eligibility criteria in the CanDoWork method of the NodeData struct. The following changes were made: - Removed the check for node active status (IsActive) - Removed the worker timeout check - Retained the check for staked status (IsStaked) The method now only considers if a node is staked and configured for the specific worker type when determining eligibility. This change allows for more inclusive worker participation, as nodes are no longer excluded based on active status or timeout conditions.
|
PR description is too short and seems to not fulfill PR template, please fill in |
⚠ golint failed (.)Found 191 lint suggestions; failing. Show Detail |
⚠ goimports failed (.)
|
⚠ gosec failed (.)Show Detail |
⚠ errcheck failed (.) |
⚠ staticcheck failed (.) |
|
PR description is too short and seems to not fulfill PR template, please fill in |
⚠ goimports failed (.)
|
⚠ golint failed (.)Found 191 lint suggestions; failing. Show Detail |
⚠ errcheck failed (.) |
⚠ shadow failed (.) |
⚠ gosec failed (.)Show Detail |
⚠ shadow failed (.) |
⚠ staticcheck failed (.) |
Replaced the multiaddress-based peer information retrieval with a DHT lookup to simplify the process. Removed unnecessary imports and optimized the code for finding and connecting to peers in the Distributed Hash Table (DHT).
|
PR description is too short and seems to not fulfill PR template, please fill in |
⚠ golint failed (.)Found 193 lint suggestions; failing. Show Detail |
⚠ goimports failed (.)
|
⚠ gosec failed (.)Show Detail |
⚠ errcheck failed (.) |
⚠ shadow failed (.) |
⚠ staticcheck failed (.) |
|
PR description is too short and seems to not fulfill PR template, please fill in |
- Modified ScrapeTweetsByQuery to immediately return rate limit errors - Updated TwitterQueryHandler to properly propagate scraper errors - Adjusted handleWorkResponse to correctly handle and return errors to the client - Ensured rate limit errors are logged and returned with appropriate HTTP status codes - Improved error message clarity for better debugging and user feedback This commit enhances the system's ability to detect, log, and respond to Twitter API rate limit errors, providing clearer feedback to both developers and end-users when such limits are encountered.
|
PR description is too short and seems to not fulfill PR template, please fill in |
- Deleted the publishWorkRequest function from pkg/api/handlers_data.go - This function was not being used in the current codebase - Removing it simplifies the code and reduces maintenance overhead
|
PR description is too short and seems to not fulfill PR template, please fill in |
- Decompose handleWorkResponse into smaller, focused functions - Introduce higher-order function for error response handling - Separate concerns for improved modularity and testability - Reduce mutable state and side effects where possible - Maintain idiomatic Go while incorporating functional principles - Improve error handling granularity and response structure
|
PR description is too short and seems to not fulfill PR template, please fill in |
|
@restevens402 @mudler this is huge! I am sending this to |
mudler
added a commit
that referenced
this pull request
Aug 26, 2024
Regression in tests introduced in #524 Signed-off-by: mudler <[email protected]>
1 task
mudler
added a commit
that referenced
this pull request
Aug 26, 2024
Regression in tests introduced in #524 Signed-off-by: mudler <[email protected]>
This was referenced Aug 26, 2024
Closed
mudler
pushed a commit
that referenced
this pull request
Sep 17, 2024
* Refactor network address handling and improve IP retrieval Restructure how multiaddresses and IP addresses are handled within the codebase to improve maintainability and efficiency. Added logic for obtaining GCP external IP addresses and restructured public address retrieval. Moved HTTP client functionality to a new package and updated references accordingly. * Update max remote workers * Improve worker selection resilience - Handle invalid multiaddresses gracefully - Continue searching for eligible workers on errors - Add more detailed logging for debugging - Prevent potential nil pointer dereference - Log warning if no workers found * Remove IsActive and timeout checks from CanDoWork method This commit simplifies the worker eligibility criteria in the CanDoWork method of the NodeData struct. The following changes were made: - Removed the check for node active status (IsActive) - Removed the worker timeout check - Retained the check for staked status (IsStaked) The method now only considers if a node is staked and configured for the specific worker type when determining eligibility. This change allows for more inclusive worker participation, as nodes are no longer excluded based on active status or timeout conditions. * extend context deadline timeout for a connection attempt * Add `MergeMultiaddresses` method and update nodes management Introduce a `MergeMultiaddresses` method to `NodeData` to handle multiaddress management more efficiently. Update the oracle node logic to use this method for merging incoming multiaddresses instead of replacing them, and add a new `NewWorker` function to initialize workers, enhancing logging and error handling for multiaddress processing. * Switch to DHT for peer address lookup Replaced the multiaddress-based peer information retrieval with a DHT lookup to simplify the process. Removed unnecessary imports and optimized the code for finding and connecting to peers in the Distributed Hash Table (DHT). * Improve Twitter API rate limit error handling and propagation - Modified ScrapeTweetsByQuery to immediately return rate limit errors - Updated TwitterQueryHandler to properly propagate scraper errors - Adjusted handleWorkResponse to correctly handle and return errors to the client - Ensured rate limit errors are logged and returned with appropriate HTTP status codes - Improved error message clarity for better debugging and user feedback This commit enhances the system's ability to detect, log, and respond to Twitter API rate limit errors, providing clearer feedback to both developers and end-users when such limits are encountered. * chore: remove unused publishWorkRequest function - Deleted the publishWorkRequest function from pkg/api/handlers_data.go - This function was not being used in the current codebase - Removing it simplifies the code and reduces maintenance overhead * refactor: handleWorkResponse with functional programming concepts - Decompose handleWorkResponse into smaller, focused functions - Introduce higher-order function for error response handling - Separate concerns for improved modularity and testability - Reduce mutable state and side effects where possible - Maintain idiomatic Go while incorporating functional principles - Improve error handling granularity and response structure --------- Co-authored-by: Bob Stevens <[email protected]>
mudler
added a commit
that referenced
this pull request
Sep 17, 2024
Regression in tests introduced in #524 Signed-off-by: mudler <[email protected]>
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.
Description
This PR fixes a critical bug where a nodes multiaddr is not correctly constructed/formed. It introduces several improvements to our network operations and worker selection process:
Network Address Handling:
GetPriorityAddressto better handle public and private addressesDHT and Peer Discovery:
WithDhtfunction to useNodeDatafor more comprehensive peer informationWorker Selection:
GetEligibleWorkersto use DHT for finding peer informationError Handling and Logging:
Code Structure:
Performance:
Artifact Handling:
These changes aim to improve the robustness and efficiency of our network operations, especially in peer discovery and worker selection. The refactoring also enhances code organization and error handling across the project.
Testing:
Notes for Reviewers
Tested initially and handles current edge cases well even when remote worker node is behind a firewall with no open ports :)
Signed commits