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

[Feature Request] support geth docker image #50

Open
yxliang01 opened this issue Jan 13, 2019 · 10 comments
Open

[Feature Request] support geth docker image #50

yxliang01 opened this issue Jan 13, 2019 · 10 comments

Comments

@yxliang01
Copy link

I think it would be cool if this package does not only support interfacing with geth as a subprocess, but also geth running in a docker container using the official geth image!

Thanks

Cute Animal Picture

From https://github.com/jaseg/lolcat
pic

pacrob pushed a commit to pacrob/py-geth that referenced this issue May 24, 2023
@0x0elliot
Copy link

Hey! I can help out with this issue if i get a simple green flag. Let me take this up? :)

@pacrob
Copy link
Contributor

pacrob commented Jan 8, 2024

Thanks, @0x0elliot , we'd appreciate the help. Consider yourself green-flagged.

@0x0elliot
Copy link

0x0elliot commented Jan 8, 2024

Alright, Let me revisit this issue this weekend :)

A head's up on where to look and what files to edit would be really cool meanwhile. I will figure this out anyways when I come back to it.

@0x0elliot
Copy link

@pacrob ^

@0x0elliot
Copy link

0x0elliot commented Jan 13, 2024

interesting. the source code seems heavily dependent on directories and commands being present. so to add docker support, the easiest way to go is to add a docker=true flag with a bunch of appropriate volumes (if necessary) to make sure that those files are always there and thus, everything works.

the first step seems like converting every command to a docker exec command which uses the docker geth binary to do the necessary.

would make my first draft. might be a bit buggy, hoping for eventual completeness with time :)

i would also take this feature very slowly, hopefully with the help of one of the maintainers!
here is how this integration would go:

  • Add Install support (build + download)
  • Add start support
  • Add stop support
  • Rest of the commands.. (slow and steady, towards a stable feature set)

@0x0elliot
Copy link

#180 to keep track of changes

@0x0elliot
Copy link

would appreciate continuous feedback on this PR @pacrob :)

this is going to be a long one. i also don't want to wait too long before our first merge. any strategy? my idea is to slowly add docker support with each PR for a certain set of features. a milestone based suggestion from you would help out a bunch :)

@0x0elliot
Copy link

0x0elliot commented Jan 14, 2024

this PR for ready for basic feedback now! i will continue post a basic review next weekend.

@pacrob
Copy link
Contributor

pacrob commented Jan 17, 2024

Hey @0x0elliot , thanks for getting this going. I'm going through the PR now. My first thought is indeed "wow, this is going to be bigger than I thought", but I don't have any better ideas at the moment. I'll think on it and get back to you.

It'll need to stay a feature branch as long as any tests aren't passing, but as long as it's regularly rebased that shouldn't be a huge problem.

@0x0elliot
Copy link

0x0elliot commented Jan 20, 2024

it's weekend again. getting to the rest of the features today. one quick thing though @pacrob

i don't think it's good to let it stay in the feature branch for too long after all tests are passing. better to release unstable broken things to the community so we have a bunch of feedback to work off of than to procrastinate on release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants