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

fix: fix for docker issue AttributeError: module 'rlp' has no attribute 'Serializable' #891

Merged
merged 4 commits into from
Jul 22, 2022

Conversation

sabotagebeats
Copy link
Contributor

@sabotagebeats sabotagebeats commented Jul 19, 2022

What I did

update dockerfile with the following:

  • update ubuntu to 22.04
  • update python to 3.9
  • change install commands for ape
  • change install commands for ape plugins
  • add RUN pip install eth-rlp==0.3.0

fixes: #857

How I did it

I tested each of the changes and all were required to successfully build and run the docker image with ape.

How to verify it

fetch & checkout this repo/branch and docker build . then run the resulting hash with docker run <hash> and it will no longer fail with the error AttributeError: module 'rlp' has no attribute 'Serializable'

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

Comment on lines -9 to +10
RUN pip install .[recommended-plugins]
RUN pip install .
RUN ape plugins install alchemy ens etherscan foundry hardhat infura ledger solidity template tokens trezor vyper
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this change? Does it function without this update? This is helpful because we can add/remove packages to the "Recommended" list without having to edit this file

Copy link
Contributor Author

@sabotagebeats sabotagebeats Jul 19, 2022

Choose a reason for hiding this comment

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

no it doesn't. If I revert there is an error when building that it can't install the plugins properly. I will update this comment shortly with the error.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fubuloubu Things tend to work better across the board when installing separately versus using setup.py keys

Copy link
Contributor Author

@sabotagebeats sabotagebeats Jul 19, 2022

Choose a reason for hiding this comment

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

=> => # INFO: This is taking longer than usual. You might need to provide the dependency resolver with stricter constraints to reduce => => # runtime. See https://pip.pypa.io/warnings/backtracking for guidance. If you want to abort this run, press Ctrl + C.
and

#11 124.5 ERROR: Cannot install eth-ape and eth-ape[recommended-plugins]==0.3.6.dev13+g5d1daf9f.d20220719 because these package versions have conflicting dependencies.
#11 124.5 The conflict is caused by:
#11 124.5     eth-ape[recommended-plugins] 0.3.6.dev13+g5d1daf9f.d20220719 depends on eth-ape 0.3.6.dev13+g5d1daf9f.d20220719 (from /code)
#11 124.5     ape-alchemy 0.2.0 depends on eth-ape<0.3.0 and >=0.2.1
#11 124.5     eth-ape[recommended-plugins] 0.3.6.dev13+g5d1daf9f.d20220719 depends on eth-ape 0.3.6.dev13+g5d1daf9f.d20220719 (from /code)
#11 124.5     ape-alchemy 0.1.1 depends on eth-ape<0.3.0 and >=0.2.1
#11 124.5     eth-ape[recommended-plugins] 0.3.6.dev13+g5d1daf9f.d20220719 depends on eth-ape 0.3.6.dev13+g5d1daf9f.d20220719 (from /code)
#11 124.5     ape-alchemy 0.1.0 depends on eth-ape<0.2.0 and >=0.1.0
#11 124.5 
#11 124.5 To fix this you could try to:
#11 124.5 1. loosen the range of package versions you've specified
#11 124.5 2. remove package versions to allow pip attempt to solve the dependency conflict
#11 124.5 
#11 124.5 ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts
------
executor failed running [/bin/sh -c pip install .[recommended-plugins]]: exit code: 1

Copy link
Contributor

@antazoey antazoey Jul 19, 2022

Choose a reason for hiding this comment

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

The plugins needs eth-ape and eth-ape needs the plugins, the cyclic dependency causes pip to have to work really hard (in the best case scenario that it succeeds)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this seems like a nasty bug!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this seems like a nasty bug!

I think it might actually be two bugs because the INFO one with the cyclic dependencies happens when I do that command in regular ape (non-docker) and although it doesn't seem to fail it takes forever to install.

RUN pip install .[recommended-plugins]
RUN pip install .
RUN ape plugins install alchemy ens etherscan foundry hardhat infura ledger solidity template tokens trezor vyper
RUN pip install eth-rlp==0.3.0
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this package seems like a stub. Do you know where this dependency is required from?

Copy link
Contributor Author

@sabotagebeats sabotagebeats Jul 19, 2022

Choose a reason for hiding this comment

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

it is required by eth-account but not listed correctly in the pins for the version we are using.

Copy link
Contributor

@antazoey antazoey Jul 19, 2022

Choose a reason for hiding this comment

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

maybe we could try adding this to our setup.py as a dependency instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we could try adding this to our setup.py as a dependency instead

it conflicts with a number of other dependencies of our current pinned version dependencies however so we would need to update the versions for those pins to be compatible as well.

@antazoey antazoey self-requested a review July 19, 2022 20:27
Copy link
Contributor

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

LGTM

honestly, I am not sure I trust [recommended-plugins] key in setup.py. Have we properly tested this in a fresh env? I think it works ok when you already have ape installed but I am concerned about the cyclic dependency and causing pip to work really hard.

@sabotagebeats
Copy link
Contributor Author

also at some point it would be good to have automated testing for docker but it would increase our check build times in github check CI

@fubuloubu
Copy link
Member

also at some point it would be good to have automated testing for docker but it would increase our check build times in github check CI

It doesn't get checked in PRs, but the deploy step is tracked on the main branch:
image

@sabotagebeats
Copy link
Contributor Author

sabotagebeats commented Jul 19, 2022

also at some point it would be good to have automated testing for docker but it would increase our check build times in github check CI

It doesn't get checked in PRs, but the deploy step is tracked on the main branch: image

Yes, agreed, however this doesn't run anything with docker it just sees if it builds or not. This is why it didn't detect this error.

Maybe we could have a seperate check that just builds and tests docker outside of pytest suite?

@fubuloubu
Copy link
Member

also at some point it would be good to have automated testing for docker but it would increase our check build times in github check CI

It doesn't get checked in PRs, but the deploy step is tracked on the main branch: image

Yes, agreed, however this doesn't run anything with docker it just sees if it builds or not. This is why it didn't detect this error.

The docker image builds okay, but fails to run?

@sabotagebeats
Copy link
Contributor Author

sabotagebeats commented Jul 19, 2022

also at some point it would be good to have automated testing for docker but it would increase our check build times in github check CI

It doesn't get checked in PRs, but the deploy step is tracked on the main branch: image

Yes, agreed, however this doesn't run anything with docker it just sees if it builds or not. This is why it didn't detect this error.

The docker image builds okay, but fails to run?

yes, this is why it didn't show up in our docker checks.

@sabotagebeats
Copy link
Contributor Author

On some level even if we do future work on this, this PR can be merged to resolve the issue and we can figure the rest out in a spike? @fubuloubu @unparalleled-js

@antazoey
Copy link
Contributor

The docker image builds okay, but fails to run?

Exactly, the error we were seeing didn't show up until you attempted to run any ape command, including ape by itself.
What we can do is make sure running the image exits with code 0 (what happens when you just run ape by itself), and if it doesn't, we fail to publish the image. This will make every published image for sure be able to be called.

@sabotagebeats
Copy link
Contributor Author

#901

@antazoey antazoey merged commit 8086fa1 into ApeWorX:main Jul 22, 2022
@sabotagebeats sabotagebeats added category: bug Something isn't working size: 3 pieces Moderately challenging, well-defined, may require a bit of research labels Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug Something isn't working size: 3 pieces Moderately challenging, well-defined, may require a bit of research
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker Startup Error: AttributeError: module 'rlp' has no attribute 'Serializable'
3 participants