Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Move logic to http-protocol and primitives packages #1140

Closed
josecelano opened this issue Dec 18, 2024 · 2 comments · Fixed by #1180
Closed

Move logic to http-protocol and primitives packages #1140

josecelano opened this issue Dec 18, 2024 · 2 comments · Fixed by #1180
Assignees
@josecelano
Labels
Code Cleanup / Refactoring Tidying and Making Neat

Comments

@josecelano
Copy link
Member

josecelano commented Dec 18, 2024 •
edited
Loading

Parent issue: #753
Relates to: #1097 (comment)

Some doc tests were very slow because they compile the main library.

They were disabled, but the right solution would be to move them to workspace packages so you don't need to compile the main lib.

Moving that code to packages (pre-existing or new packages) was something planned anyway because it makes possible to share that code with other projects and compilation should also be faster (you don't need to recompile them if you don't touch anything inside those packages).

New packages (dir and crate name):

  • packages/http-protocol (bittorrent-http-protocol)
  • packages/tracker-core (torrust-tracker-core)

Modules to extract:

To http-protocol:

  • src/servers/http/v1/query.rs
  • src/servers/http/v1/requests/announce.rs
  • src/servers/http/v1/responses/announce.rs
  • src/servers/http/v1/responses/error.rs
  • src/servers/http/v1/responses/scrape.rs
  • src/servers/http/v1/services/peer_ip_resolver.rs (I'm not sure about this)

To tracker-core:

  • crate::core::AnnounceData
  • crate::core::ScrapeData
    ...

See https://github.com/torrust/torrust-tracker/pull/1141/files
@josecelano josecelano added the Code Cleanup / Refactoring Tidying and Making Neat label Dec 18, 2024
@josecelano josecelano self-assigned this Dec 18, 2024
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Dec 18, 2024
@josecelano
fix: [1097] do not compile slow doc tests
0bbe83b 
It takes 32 seconds to compile and run all doc tests inckuding the ones
that use the main tracker lib (each test compiles the main lib).

This change disabled the compilation and execution of those tests.

In terms of code coverage is not a problem becuase we have units test
for that functionality. It only affects to the documentation. However
there are too slow and they are even making crah some developers'
computers.

The good solution would be to extract those parts into new workspace
packages to avoid compiling the main library. A new issue has been
opened for it:

torrust#1140
@josecelano josecelano mentioned this issue Dec 18, 2024
Merged
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Dec 18, 2024
@josecelano
fix: [1097] do not compile slow doc tests
0e13ff7 
It takes 32 seconds to compile and run all doc tests inckuding the ones
that use the main tracker lib (each test compiles the main lib).

This change disabled the compilation and execution of those tests.

In terms of code coverage is not a problem becuase we have units test
for that functionality. It only affects to the documentation. However
there are too slow and they are even making crah some developers'
computers.

The good solution would be to extract those parts into new workspace
packages to avoid compiling the main library. A new issue has been
opened for it:

torrust#1140
josecelano added a commit that referenced this issue Dec 18, 2024
@josecelano
Merge #1141: Don't execute slow doc tests
df101fb 
0e13ff7 fix: [1097] do not compile slow doc tests (Jose Celano)

Pull request description:

  This PR disables slow doc tests. They are slow because they include the main library, and each test has to compile it.

  They will be enabled again after [moving the code to workspace sub-packages](#1140) that do not depend on the main tracker lib.

ACKs for top commit:
  josecelano:
    ACK 0e13ff7

Tree-SHA512: fc5542deaabea3c66c4b6994736e89470052dac90db7bd4739c3033aa6de81a6de62c3e37d793f05524dbb92328c3683b7f4fd615303a0839e00829bf500dec4
@josecelano josecelano changed the title Extract more logic to the http-protocol package Extract more packages: http-protocol and tracker-core packages Jan 10, 2025
@josecelano josecelano changed the title Extract more packages: http-protocol and tracker-core packages Move logic to package http-protocol Jan 11, 2025
@josecelano
Copy link
Member Author

The package http-protocol was already created. We only need to move the logic there and extract the new tracker-core package.

josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 11, 2025
@josecelano
refactor: [torrust#1140] move http query to http-protocol package
b0b780a
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 11, 2025
@josecelano
refactor: [torrust#1140] move http tracker logic to http-protocol and…
9494477 
… primitives packages

- Generic logic for http tracker has bben moved to http-protocol package (bittorrent-http-protocol crate).
- Generic tracker types like AnnounceData and ScrapeData have been moved
  to the primitives package (torrust-tracker-primitives crate).

This has also a desiderable side effect: generci re-usable domain logic
has been decoupled from Axum framework.
@josecelano josecelano linked a pull request Jan 11, 2025 that will close this issue
Refactor: Move logic to http-protocol and primitives packages #1180
Merged
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 11, 2025
@josecelano
refactor: [torrust#1140] move http tracker logic to http-protocol and…
a7ceb0f 
… primitives packages

- Generic logic for http tracker has bben moved to http-protocol package (bittorrent-http-protocol crate).
- Generic tracker types like AnnounceData and ScrapeData have been moved
  to the primitives package (torrust-tracker-primitives crate).

This has also a desiderable side effect: generic re-usable domain logic
has been decoupled from Axum framework.
@josecelano josecelano changed the title Move logic to package http-protocol Move logic to http-protocol and primitives packages Jan 11, 2025
@josecelano
Copy link
Member Author

The package http-protocol was already created. We only need to move the logic there and extract the new tracker-core package.

I did not create the tracker-core package because I have only moved basic types. I moved them to the primitives package.

josecelano added a commit that referenced this issue Jan 11, 2025
@josecelano
Merge #1180: Refactor: Move logic to http-protocol and primitives…
b403382 
… packages

c2d134e test: re-enable slow tests (Jose Celano)
a7ceb0f refactor: [#1140] move http tracker logic to http-protocol and primitives packages (Jose Celano)

Pull request description:

  Refactor: Move logic to `http-protocol` and `primitives` packages. This will:

  - Allow sharing that code with other projects.
  - Allow re-enabling doc tests disabled [here](#1097).
  - Decouple generic reusable logic from Axum framework.
  - Improve compilation times.

ACKs for top commit:
  josecelano:
    ACK c2d134e

Tree-SHA512: 00c49567aed10f23e525219a26765ca2e8ce948037785da7622076e7d6de981983cbf3c88f6043631ac7bfc2def2d3161dc954a8f77746c4be48595d09af49ad
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@josecelano josecelano

Labels
Code Cleanup / Refactoring Tidying and Making Neat
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor: Move logic to http-protocol and primitives packages josecelano/torrust-tracker
1 participant
@josecelano