Skip to content
This repository was archived by the owner on Jan 27, 2026. It is now read-only.

Add software check for miner port#240

Merged
JannikSt merged 2 commits into
PrimeIntellect-ai:developfrom
outwrit:fix/check-miner-port
Apr 21, 2025
Merged

Add software check for miner port#240
JannikSt merged 2 commits into
PrimeIntellect-ai:developfrom
outwrit:fix/check-miner-port

Conversation

@outwrit
Copy link
Copy Markdown
Contributor

@outwrit outwrit commented Apr 10, 2025

Fixes #63

@JannikSt
Copy link
Copy Markdown
Member

Thank you for your contribution! Can you run cargo fmt and also cargo clippy --fix --allow-dirty?

@JannikSt JannikSt requested a review from Copilot April 11, 2025 17:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

worker/src/docker/service.rs:105

  • Ensure that the use of the 'is_none_or' method is supported in your Rust version or runtime; if not, consider using a more stable alternative such as chain-mapping or a custom helper function.
.as_ref().is_none_or(|id| !c.names.iter().any(|name| name.contains(id)))

worker/src/checks/software/port.rs:17

  • Acquiring a read lock prevents mutations; if 'add_issue' requires mutable access, switch to a write lock (e.g. using issues.write().await) before calling add_issue.
let issue_tracker = issues.read().await;

@JannikSt JannikSt force-pushed the fix/check-miner-port branch from fa908fe to 1548f65 Compare April 21, 2025 15:28
@JannikSt JannikSt requested a review from Copilot April 21, 2025 15:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the issue of the miner port check by introducing a dedicated software check for port availability and refactoring the software checking mechanism. Key changes include:

  • Updating DockerService filtering to use the new is_none_or method.
  • Replacing the legacy run_software_check function with a new SoftwareChecker struct.
  • Implementing a new port check in the software module and adding a corresponding IssueType for port unavailability.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
worker/src/docker/service.rs Refactored container filtering logic using is_none_or for task identification.
worker/src/cli/command.rs Updated command execution flow to use the new SoftwareChecker and refined error handling.
worker/src/checks/software/software_check.rs Introduced SoftwareChecker struct and integrated port availability check.
worker/src/checks/software/port.rs Added a new function to check port availability with potential issues in lock usage.
worker/src/checks/software/mod.rs Exposed the SoftwareChecker and port modules.
worker/src/checks/issue.rs Added new IssueType for PortUnavailable to support the port check.

}

pub async fn check_port_available(issues: &Arc<RwLock<IssueReport>>, port: u16) -> Result<()> {
let issue_tracker = issues.read().await;
Copy link

Copilot AI Apr 21, 2025

Choose a reason for hiding this comment

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

Mutating the issue tracker via add_issue on a read lock may lead to runtime issues. Consider replacing the read() call with write() to obtain a mutable lock when adding an issue.

Suggested change
let issue_tracker = issues.read().await;
let mut issue_tracker = issues.write().await;

Copilot uses AI. Check for mistakes.
@JannikSt JannikSt merged commit 1e0a4dc into PrimeIntellect-ai:develop Apr 21, 2025
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check if worker port is actually available in software checks

3 participants