Rename `tracker` mod to `core` #255

josecelano · 2023-03-20T19:05:43Z

I propose renaming the root module tracker to core.

Mod: https://github.com/torrust/torrust-tracker/tree/develop/src/tracker

$ tree src/core/
src/core/
├── auth.rs
├── databases
│   ├── driver.rs
│   ├── error.rs
│   ├── mod.rs
│   ├── mysql.rs
│   └── sqlite.rs
├── error.rs
├── mod.rs
├── peer.rs
├── services
│   ├── mod.rs
│   ├── statistics
│   │   ├── mod.rs
│   │   └── setup.rs
│   └── torrent.rs
├── statistics.rs
├── torrent.rs
└── tracker.rs

That will allow having these namespaces:

core::tracker::Tracker
core::peer::Peer
Etcetera.

In my opinion, that's better than the:

tracker::Tracker
tracker::peer::Peer

The mod contains more things than just the tracker, and that Tracker could even disappear in the future. I think what we have is the domain logic for BitTorrent trackers, regardless of which delivery mechanism you use (the way end-user interact with that domain logic).

I'm writing the documentation for the crate, and it makes sense to give it a name to the "core" tracker logic.

If we move the mod to a package in the future, the name would be torrust-tracker-tracker if we follow current conventions. I think torrust-tracker-core would be a better name.

I'm open to more options. I think core may be too generic.

Other alternatives:

torrust-tracker-domain
torrust-bittorrent-tracker

We must consider that we also have another package called shared with logic used by the core, http, udp, ...

cc @da2ce7 @WarmBeer

The text was updated successfully, but these errors were encountered:

da2ce7 · 2023-03-20T19:28:30Z

Sounds reasonable to me :)

josecelano · 2023-03-21T09:01:16Z

Hi @da2ce7, I've been giving more thought to this.

These are the packages we have extracted so far:

packages/
├── configuration
├── located-error
├── primitives
└── test-helpers

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      <- NEW!: torrust-tracker-bittorrent (shared)
├── clock           <- NEW!: torrust-tracker-clock (shared)
├── crypto          <- NEW!: torrust-tracker-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.

josecelano · 2023-03-21T09:07:25Z

And maybe we should rename some of the packages I've mentioned avobe:

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

I would like the packages to match the config.toml file sections:

log_level = "debug"

[core_tracker]
...

[[udp_trackers]]
...

[[http_trackers]]
...

[http_api]
... 

[http_api.access_tokens]
...

josecelano · 2023-03-21T09:12:52Z

Should we remove the traker prefix from some package names?

packages/
├── http-api        <- torrust-tracker-http-api
├── udp-tracker     <- torrust-udp-tracker
├── http-tracker    <- torrust-http-tracker
├── core-tracker    <- torrust-core-tracker
├── bittorrent      <- torrust-bittorrent
├── clock           <- torrust-clock
├── crypto          <- torrust-crypto
├── configuration   <- torrust-tracker-configuration
├── located-error   <- torrust-tracker-located-error
├── primitives      <- torrust-tracker-primitives
└── test-helpers    <- torrust-tracker-test-helpers

da2ce7 · 2023-03-21T09:55:42Z

@josecelano https://github.com/greatest-ape/aquatic uses fully-referenced folder names. However I don't think that it is needed necessarily...

josecelano · 2023-03-21T10:20:31Z

@josecelano https://github.com/greatest-ape/aquatic uses fully-referenced folder names. However I don't think that it is needed necessarily...

Hi @da2ce7, I'm ok with keeping the folder names shorter (left side) and different from the crate's name.

Notice that on the right side, I wrote the cargo crate names. With this reorganization, we have two main advantages:

Other projects can start reusing those packages (even our index project)
We avoid crate name conflicts or ambiguity in the future with a good structure now.
We force contributors to keep them decoupled.

But as I said, we can postpone it. Until now, the reason to extract a package has always been to resolve cycle dependencies and not to reuse or decouple components. Maybe that could be a goal for version 4.0.0.

josecelano · 2023-12-01T06:59:09Z

Hey @WarmBeer @mario-nt what do you think? I think this is important for the long term, although I would not do it before the next major release v3.0.0.

mario-nt · 2023-12-02T23:20:36Z

@josecelano

I like the idea of changing the name from tracker to core, it might be generic but I think it is effective and self explanatory. Also less redundant, as we are not using the word tracker so many times.

I also like keeping the packages's names shorter and keeping the different configuration options separated.

Leaving these changes to the next major release is a good idea, as they are important but don´t affect how the app works or it's performance and there are more important feats/bugs/etc to focus on for now.

da2ce7 · 2023-12-03T02:41:43Z

@josecelano and @mario-nt I think that it is a simple refactor, should be a couple of hours work at most; It could be done anytime.

josecelano added Enhancement / Feature Request Something New Code Cleanup / Refactoring Tidying and Making Neat labels Mar 20, 2023

da2ce7 added Documentation Improves Instructions, Guides, and Notices - Contributor - Nice to support Torrust Needs Feedback What dose the Community Think? and removed Enhancement / Feature Request Something New labels Oct 10, 2023

da2ce7 mentioned this issue Dec 3, 2023

dev: refactor: rename tracker mod to core #531

Merged

josecelano closed this as completed in e6aecf1 Dec 4, 2023

josecelano mentioned this issue Feb 9, 2024

Extract Tracker API Client packages #689

Open

josecelano mentioned this issue Mar 25, 2024

Refactor packages #753

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename `tracker` mod to `core` #255

Rename `tracker` mod to `core` #255

josecelano commented Mar 20, 2023

da2ce7 commented Mar 20, 2023

josecelano commented Mar 21, 2023 •

edited

josecelano commented Mar 21, 2023 •

edited

josecelano commented Mar 21, 2023

da2ce7 commented Mar 21, 2023

josecelano commented Mar 21, 2023

josecelano commented Dec 1, 2023

mario-nt commented Dec 2, 2023

da2ce7 commented Dec 3, 2023

Rename tracker mod to core #255

Rename tracker mod to core #255

Comments

josecelano commented Mar 20, 2023

da2ce7 commented Mar 20, 2023

josecelano commented Mar 21, 2023 • edited

josecelano commented Mar 21, 2023 • edited

josecelano commented Mar 21, 2023

da2ce7 commented Mar 21, 2023

josecelano commented Mar 21, 2023

josecelano commented Dec 1, 2023

mario-nt commented Dec 2, 2023

da2ce7 commented Dec 3, 2023

Rename `tracker` mod to `core` #255

Rename `tracker` mod to `core` #255

josecelano commented Mar 21, 2023 •

edited

josecelano commented Mar 21, 2023 •

edited