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

Refactor packages #753

Open
josecelano opened this issue Mar 25, 2024 · 0 comments
Open

Refactor packages #753

josecelano opened this issue Mar 25, 2024 · 0 comments
Assignees
Labels
Code Cleanup / Refactoring Tidying and Making Neat Needs Feedback What dose the Community Think?

Comments

@josecelano
Copy link
Member

josecelano commented Mar 25, 2024

From: #255 (comment)

UPDATED: 2024-11-27. List of current packages.

These are the packages we have extracted so far in the tracker:

packages/
├── clock               -> torrust-tracker-clock
├── configuration       -> torrust-tracker-configuration
├── located-error       -> torrust-tracker-located-error
├── primitives          -> torrust-tracker-primitives
├── test-helpers        -> torrust-tracker-test-helpers
├── torrent-repository  -> torrust-tracker-torrent-repository
└── tracker-client      -> bittorrent-tracker-client

console/
└── tracker-client      -> torrust-tracker-client

And other independent packages:

torrust/bittorrent-primitives -> bittorrent-primitives

In the long-term I see we could have the following:

packages/
├── api             <- NEW!: torrust-tracker-api (servers)
├── udp             <- NEW!: torrust-tracker-udp (servers)
├── http            <- NEW!: torrust-tracker-http (servers)
├── core            <- NEW!: torrust-tracker-core (tracker)
├── bittorrent      <-       bittorrent-primitives
├── clock           <-       torrust-clock (shared)
├── crypto          <- NEW!: torrust-crypto (shared)
├── configuration   <-       torrust-tracker-configuration
├── located-error   <-       torrust-tracker-located-error
├── primitives      <-       torrust-tracker-primitives
└── test-helpers    <-       torrust-tracker-test-helpers

The "shared" ones could be a single package, but as they do not have much in common and could be used independently, I would keep them in separate packages.

I would also refactor the config file from:

log_level = "debug"
mode = "public"
db_driver = "Sqlite3"
db_path = "./storage/database/data.db"
announce_interval = 120
min_announce_interval = 120
max_peer_timeout = 900
on_reverse_proxy = false
external_ip = "2.137.87.41"
tracker_usage_statistics = true
persistent_torrent_completed_stat = true
inactive_peer_cleanup_interval = 600
remove_peerless_torrents = false

[[udp_trackers]]
enabled = true
bind_address = "0.0.0.0:6969"

[[http_trackers]]
enabled = true
bind_address = "0.0.0.0:7070"
ssl_enabled = false
ssl_cert_path = "./storage/ssl_certificates/localhost.crt"
ssl_key_path = "./storage/ssl_certificates/localhost.key"

[http_api]
enabled = true
bind_address = "0.0.0.0:1212"
ssl_enabled = false
ssl_cert_path = "./storage/ssl_certificates/localhost.crt"
ssl_key_path = "./storage/ssl_certificates/localhost.key"

[http_api.access_tokens]
admin = "MyAccessToken"

to:

log_level = "debug"

[core_tracker]
mode = "public"
db_driver = "Sqlite3"
db_path = "./storage/database/data.db"
announce_interval = 120
min_announce_interval = 120
max_peer_timeout = 900
on_reverse_proxy = false
external_ip = "2.137.87.41"
tracker_usage_statistics = true
persistent_torrent_completed_stat = true
inactive_peer_cleanup_interval = 600
remove_peerless_torrents = false

[[udp_trackers]]
enabled = true
bind_address = "0.0.0.0:6969"

[[http_trackers]]
enabled = true
bind_address = "0.0.0.0:7070"
ssl_enabled = false
ssl_cert_path = "./storage/ssl_certificates/localhost.crt"
ssl_key_path = "./storage/ssl_certificates/localhost.key"

[http_api]
enabled = true
bind_address = "0.0.0.0:1212"
ssl_enabled = false
ssl_cert_path = "./storage/ssl_certificates/localhost.crt"
ssl_key_path = "./storage/ssl_certificates/localhost.key"

[http_api.access_tokens]
admin = "MyAccessToken"

The configuration will also change accordingly from:

pub struct Configuration {
    pub log_level: Option<String>,
    pub mode: TrackerMode,
    pub db_driver: DatabaseDriver,
    pub db_path: String,
    pub announce_interval: u32,
    pub min_announce_interval: u32,
    pub max_peer_timeout: u32,
    pub on_reverse_proxy: bool,
    pub external_ip: Option<String>,
    pub tracker_usage_statistics: bool,
    pub persistent_torrent_completed_stat: bool,
    pub inactive_peer_cleanup_interval: u64,
    pub remove_peerless_torrents: bool,
    pub udp_trackers: Vec<UdpTracker>,
    pub http_trackers: Vec<HttpTracker>,
    pub http_api: HttpApi,
}

to:

pub struct Configuration {
    pub log_level: Option<String>,
    pub core_tracker: CoreTracker,
    pub udp_trackers: Vec<UdpTracker>,
    pub http_trackers: Vec<HttpTracker>,
    pub http_api: HttpApi,
}

pub struct CoreTracker {
    pub mode: TrackerMode,
    pub db_driver: DatabaseDriver,
    pub db_path: String,
    pub announce_interval: u32,
    pub min_announce_interval: u32,
    pub max_peer_timeout: u32,
    pub on_reverse_proxy: bool,
    pub external_ip: Option<String>,
    pub tracker_usage_statistics: bool,
    pub persistent_torrent_completed_stat: bool,
    pub inactive_peer_cleanup_interval: u64,
    pub remove_peerless_torrents: bool,
}

I'm tempted to do those refactorings before continuing with the crate's documentation.

I would also move UdpTracker, HttpTracker and HttpApi to their crates (if possible) or even to new packages if we depend only on those types:

packages/
├── api             <- NEW!: torrust-tracker-api (servers)
├── udp             <- NEW!: torrust-tracker-udp (servers)
├── http            <- NEW!: torrust-tracker-http (servers)
├── api-config      <- NEW!: torrust-tracker-api-config
├── udp-config      <- NEW!: torrust-tracker-udp-config
├── http-config     <- NEW!: torrust-tracker-http-config
├── core-config     <- NEW!: torrust-tracker-core-config
...

I would keep the current torrust-tracker-configuration for the main tracker app configuration:

pub struct Configuration {
    pub log_level: Option<String>,
    pub core_tracker: CoreTracker,
    pub udp_trackers: Vec<UdpTracker>,
    pub http_trackers: Vec<HttpTracker>,
    pub http_api: HttpApi,
}

What do you think @da2ce7 @WarmBeer? We can do it later, but it will cost extra effort to refactor the documentation.

Regarding the config file, that would be a big change that would need to wait until the next major release (V4) if we want to do it.

Originally posted by @josecelano in #255 (comment)

@josecelano josecelano mentioned this issue Mar 25, 2024
6 tasks
@josecelano josecelano added the Code Cleanup / Refactoring Tidying and Making Neat label Mar 25, 2024
@josecelano josecelano self-assigned this Jun 19, 2024
@josecelano josecelano added the Needs Feedback What dose the Community Think? label Jun 19, 2024
@josecelano josecelano added this to the v3.0.0 milestone Jun 19, 2024
@josecelano josecelano removed this from the v3.0.0 milestone Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup / Refactoring Tidying and Making Neat Needs Feedback What dose the Community Think?
Projects
None yet
Development

No branches or pull requests

2 participants