Skip to content
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

chore:[#674] Tracker Checker: Ouput in JSON #810

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

hungfnt
Copy link
Contributor

@hungfnt hungfnt commented Apr 24, 2024

No description provided.

Copy link

codecov bot commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 78.91%. Comparing base (9a1ce0e) to head (b27f002).

Files Patch % Lines
src/console/clients/checker/checks/health.rs 0.00% 2 Missing ⚠️
src/console/clients/checker/checks/structs.rs 0.00% 2 Missing ⚠️
src/console/clients/checker/checks/http.rs 0.00% 1 Missing ⚠️
src/console/clients/checker/checks/udp.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #810      +/-   ##
===========================================
- Coverage    78.93%   78.91%   -0.03%     
===========================================
  Files          162      163       +1     
  Lines         9186     9186              
===========================================
- Hits          7251     7249       -2     
- Misses        1935     1937       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@josecelano josecelano linked an issue Apr 24, 2024 that may be closed by this pull request
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

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

Hi @ngthhu looks good. Thank you! Just a couple of things.

Fix formatting errors

https://github.com/torrust/torrust-tracker/actions/runs/8813597282/job/24196746331?pr=810

Output should be only the json

Command:

cargo run --bin tracker_checker -- --config-path "./share/default/config/tracker_checker.json"

Output:

Running checks for trackers ...
{
  "udp_trackers": [
    {
      "url": "127.0.0.1:6969",
      "status": {
        "code": "ok",
        "message": ""
      }
    }
  ],
  "http_trackers": [
    {
      "url": "http://127.0.0.1:7070/",
      "status": {
        "code": "ok",
        "message": ""
      }
    }
  ],
  "health_checks": [
    {
      "url": "http://127.0.0.1:1313/health_check",
      "status": {
        "code": "ok",
        "message": ""
      }
    }
  ]
}

Output should not include:

Running checks for trackers ...

You should be able to parse the output as a json. For example:

cargo run --bin tracker_checker -- --config-path "./share/default/config/tracker_checker.json"| jq
    Finished `dev` profile [optimized + debuginfo] target(s) in 0.09s
     Running `target/debug/tracker_checker --config-path ./share/default/config/tracker_checker.json`
parse error: Invalid numeric literal at line 1, column 8
thread 'main' panicked at library/std/src/io/stdio.rs:1118:9:
failed printing to stdout: Broken pipe (os error 32)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@hungfnt
Copy link
Contributor Author

hungfnt commented Apr 24, 2024

Is it good to name a file with a general name like structs.rs?

@josecelano
Copy link
Member

Is it good to name a file with a general name like structs.rs?

Hi @ngthhu, Since it's a small mod and file, I don't see a big problem. In my case, when I read structs, I always think they must contain a kind of DTOs.

But, I would probably have named them:

output -> the module

pub struct Output {
    pub url: String,
    pub status: Status,
}

pub struct Status {
    pub code: String,
    pub message: String,
}

Or:

output -> the module

pub struct Result {
    pub url: String,
    pub status: Status,
}

pub struct Status {
    pub code: String,
    pub message: String,
}

@josecelano
Copy link
Member

ACK b27f002

@josecelano josecelano merged commit 92349d3 into torrust:develop Apr 24, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracker Checker: Ouput in JSON
2 participants