Skip to content

Conversation

richardkiss
Copy link
Contributor

As part of the project with tach to remove cyclic dependencies, this PR removes many cyclic dependencies that include chia.server: chia.XX where XX is one of farmer, simulator, full_node, harvester, introducer, wallet, apis, seeder, timelord, data_layer. So it's a lot, but it's mostly just one trick repeated multiple times.

It also requires adding some dependencies, and yes, introducing a new cycle with chia.apis. But this one we can now target with a laser focus since it's so small (one file with just one entry: ApiProtocolRegistry.

@richardkiss richardkiss requested review from a team and altendky as code owners August 5, 2025 00:11
@richardkiss richardkiss changed the title Simplify dependencies chia.apis Simplify dependencies wrt chia.server Aug 5, 2025
@richardkiss richardkiss marked this pull request as draft August 5, 2025 00:11
@richardkiss richardkiss force-pushed the simplify-dependencies-chia.apis branch from be1f5dc to 9c7ea51 Compare August 5, 2025 00:13
@richardkiss richardkiss added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Aug 5, 2025
@richardkiss richardkiss marked this pull request as ready for review August 5, 2025 00:46
@richardkiss richardkiss requested a review from Copilot August 5, 2025 00:46
Copy link

@Copilot 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 simplifies dependencies related to chia.server by removing cyclic dependencies that include chia.server and multiple modules (farmer, simulator, full_node, harvester, introducer, wallet, apis, seeder, timelord, data_layer). The changes move service definitions from a centralized chia.server.aliases file to individual module-specific files and update entry points to reference the new locations.

  • Removes cyclic dependencies between chia.server and various service modules
  • Relocates service type definitions from centralized aliases to module-specific files
  • Updates entry points in pyproject.toml to point to service-specific modules

Reviewed Changes

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

Show a summary per file
File Description
tach.toml Removes deprecated dependencies from chia.server and adds new dependencies to specific modules
pyproject.toml Updates entry points to use module-specific start scripts instead of centralized server scripts
chia/server/aliases.py Completely removes the centralized service aliases file
Multiple service modules Creates new service definition files (e.g., wallet_service.py, timelord_service.py) with type aliases
Multiple start modules Updates imports to use local service definitions instead of centralized aliases
Test files Updates imports to reference new service locations
build_scripts/pyinstaller.spec Updates binary paths to use module-specific start scripts

Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

just a note along the way here, the start_*.py files are all very similar and ought to be deduped at some point. the only relevance here is that they are being moved further away from each other which makes it easier to forget and have them diverge. no action requested on this point.

@richardkiss richardkiss force-pushed the simplify-dependencies-chia.apis branch from 9c7ea51 to 368ba40 Compare August 5, 2025 20:39
@richardkiss
Copy link
Contributor Author

just a note along the way here, the start_*.py files are all very similar and ought to be deduped at some point. the only relevance here is that they are being moved further away from each other which makes it easier to forget and have them diverge. no action requested on this point.

I agree. I looked at this a few year ago and was very frustrated by the subtle divergences and didn't have the courage at the time to try to resolve it.

@richardkiss richardkiss force-pushed the simplify-dependencies-chia.apis branch from 9764aef to 4d0c90c Compare August 5, 2025 21:10
@richardkiss richardkiss requested a review from arvidn August 5, 2025 21:15
@richardkiss richardkiss force-pushed the simplify-dependencies-chia.apis branch from 4d0c90c to 7a5f969 Compare August 5, 2025 22:00
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

looks good. and I also want to remind everyone that we don't treat chia-blockchain as a "library" with a stable API, and this is an example of that.

@Starttoaster Starttoaster merged commit 4e6038b into Chia-Network:main Aug 6, 2025
344 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants