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

CHIA-597: Use poetry for package management #11057

Merged
merged 166 commits into from
Jul 10, 2024
Merged

CHIA-597: Use poetry for package management #11057

merged 166 commits into from
Jul 10, 2024

Conversation

altendky
Copy link
Contributor

@altendky altendky commented Apr 5, 2022

Devs should continue to use the install.sh / Install.ps1 scripts for convenience. These scripts do the following:

Download and install poetry into .penv
Setup a chia-blockchain specific .venv
Create a symlink between venv and .venv

You can then use activate in the same way as before

This approach keeps the poetry deployment in .penv separate from the chia-blockchain install in .venv. If you want to run the poetry command you will need to run it from the .penv directory.

You can install poetry globally if you want, but then you may need to pay attention to the versioning in poetry specifically (running a different version from the one installed in install.sh may have unexpected results). We are still exploring options on better ways to handle the poetry install.

Draft for:

Copy link
Contributor

File Coverage Missing Lines
chia/_tests/core/data_layer/util.py 62.5% lines 143, 160-161
Total Missing Coverage
9 lines 3 lines 66%

@emlowe emlowe marked this pull request as ready for review July 8, 2024 17:15
@emlowe emlowe requested a review from a team as a code owner July 8, 2024 17:15
@emlowe
Copy link
Contributor

emlowe commented Jul 8, 2024

@SocketSecurity ignore pypi/[email protected]
@SocketSecurity ignore pypi/[email protected]

Copy link
Contributor

github-actions bot commented Jul 8, 2024

File Coverage Missing Lines
chia/_tests/core/data_layer/util.py 62.5% lines 143, 160-161
Total Missing Coverage
9 lines 3 lines 66%

@arvidn
Copy link
Contributor

arvidn commented Jul 8, 2024

when testing this on Mac (M1) I get this warning:

Warning: The file chosen for install of twine 5.1.0 (twine-5.1.0-py3-none-any.whl) is yanked. Reason for being yanked: https://github.com/pypa/twine/issues/1125

I suppose this is related to #18253

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. It would be nice to have some documented guidance on how treat the lockfile, and PRs making changes to it. I assume it would be accepted with caution, as we want to lock down our supply-chain risk.

arvidn
arvidn previously approved these changes Jul 8, 2024
@Starttoaster
Copy link
Contributor

Waiting on coverage-diff override

@emlowe emlowe closed this Jul 8, 2024
@emlowe emlowe reopened this Jul 8, 2024
@emlowe
Copy link
Contributor

emlowe commented Jul 8, 2024

Sorry Clicked the wrong thing and ended up closing and reopening the PR

@emlowe
Copy link
Contributor

emlowe commented Jul 8, 2024

looks good. It would be nice to have some documented guidance on how treat the lockfile, and PRs making changes to it. I assume it would be accepted with caution, as we want to lock down our supply-chain risk.

Yes, generally you will want to use poetry to make any changes, eg:

.penv/bin/poetry update urllib3

However, I'm also hopeful that now all the dependencies are listed in the lock file, we can relay on dependabot creating PRs to update anything

@emlowe
Copy link
Contributor

emlowe commented Jul 8, 2024

Well, dependabot seems to have a flow in that it updates the lock files but not the toml (dependabot/dependabot-core#8603)

So that might take some manual effort per the reporter until that gets resolved....

Copy link
Contributor

github-actions bot commented Jul 8, 2024

File Coverage Missing Lines
chia/_tests/core/data_layer/util.py 62.5% lines 143, 160-161
Total Missing Coverage
9 lines 3 lines 66%

Copy link

coveralls-official bot commented Jul 8, 2024

Pull Request Test Coverage Report for Build 9860288518

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 6 of 9 (66.67%) changed or added relevant lines in 2 files are covered.
  • 39 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.03%) to 90.955%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/_tests/core/data_layer/util.py 5 8 62.5%
Files with Coverage Reduction New Missed Lines %
chia/wallet/wallet_node.py 1 88.72%
chia/_tests/util/test_build_job_matrix.py 1 96.77%
chia/daemon/client.py 1 73.94%
chia/introducer/introducer.py 1 78.26%
chia/data_layer/data_layer_wallet.py 2 91.93%
chia/server/node_discovery.py 4 79.08%
chia/server/address_manager.py 7 90.48%
chia/_tests/core/util/test_lockfile.py 22 77.73%
Totals Coverage Status
Change from base Build 9846325110: -0.03%
Covered Lines: 101594
Relevant Lines: 111650

💛 - Coveralls

Copy link
Contributor

github-actions bot commented Jul 9, 2024

File Coverage Missing Lines
chia/_tests/core/data_layer/util.py 62.5% lines 143, 160-161
Total Missing Coverage
9 lines 3 lines 66%

Copy link
Contributor

github-actions bot commented Jul 9, 2024

File Coverage Missing Lines
chia/_tests/core/data_layer/util.py 62.5% lines 143, 160-161
Total Missing Coverage
9 lines 3 lines 66%

@Starttoaster Starttoaster merged commit 6d9219b into main Jul 10, 2024
369 of 370 checks passed
@Starttoaster Starttoaster deleted the poetry branch July 10, 2024 15:47
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants