Make dependencies for Ledger support optional #1474
Conversation
|
great, we were stuck because of compatibility issues because of this. |
|
@droserasprout thanks for the PR! It looks good, will review it soon :) |
|
@droserasprout Could you please fix linting? |
|
There are multiple |
|
Ok, no worries. Will review it right after #1481 is merged, because this way "Tests" job in your PR will not be breaking. |
|
@droserasprout at first glance PR looks good, and passes tests (without network tests because env variables are not set). |
There was a problem hiding this comment.
Looking good 👍
I think that we need to install Ledger extra using poetry install -E ledger only in Tests job (ledger is tested only there), in the rest it can be removed. Could you check it please @droserasprout ?
|
Done. I've updated a couple of section headers to make SETUP PYTHON part more copypastable. |
|
@droserasprout |
franciszekjob
merged commit 2a84190
into
software-mansion:development
Introduced changes
ledgerwalletandbip-utilsdependencies made optional. To useLedgerSignerthe package needs to be installed with-E ledger.These two libraries require lots of cryptography dependencies that significantly increase size of the virtualenv, Docker images, increase installation time and require compilation on many platforms.
LateExceptionhelper ensures thatImportErrorexception is raised only when Ledger code is actually used. Lazy imports could in addition improve import times, but require much more code changes.Breaking changes
Existing users will need to add
ledgerextra to dependencies after the update.Bonus: workaround
If you're dealing with the same issues, found this PR, and it's still open, here's how to override dependencies in pyproject.toml with PDM package manager. Unfortunately, Poetry doesn't support such tricks.