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

Bump mitmproxy version to ^8 #605

Open
2 tasks
lucekdudek opened this issue Jan 17, 2023 · 1 comment
Open
2 tasks

Bump mitmproxy version to ^8 #605

lucekdudek opened this issue Jan 17, 2023 · 1 comment

Comments

@lucekdudek
Copy link
Contributor

lucekdudek commented Jan 17, 2023

Current version of mitmproxy result in goth capping dependency of click at <=8. Click is a dependency used in other golemfactory python project which makes it hard to use goth and newest version of other dependencies at the same time.

There ware bigger changes made in mitmproxy update from ^6 to ^8. Changes that we know impact goth:

           loop = asyncio.new_event_loop()
           # Monkey patch the loop to set its `add_signal_handler` method to no-op.
           # The original method would raise error since the loop will run in
           # a non-main thread and hence cannot have signal handlers installed.
           loop.add_signal_handler = lambda *args_: None
           asyncio.set_event_loop(loop)
  • mitmproxy.http.HTTPRequestand mitmproxy.http.HTTPResponse were renamed to Request and Response

What needs to be done:

  • Add more tests which will allow for safer version bumping and would allow for easier detacion of compabilities breakdows when bumping versions of dependencies
  • Bump version of mitmproxy so it supports click>=8 (note: mitmproxy=^9 requires python>=3.9 therefor bump to ^8 as for now) - this will refactoring of goth, using poe checks_typing can point some red flags in the code base as starting point.
@shadeofblue
Copy link
Contributor

what I tried:

  1. use multiprocessing instead of threading to alleviate the issue with the event loop threads - no success - either with the new version or after a fallback to the current one...

  2. tried a more direct approach

    • ported references to use Request, Response
    • mocked the signal calls

still - no dice...

General conclusions after trying to debug why the above approaches didn't work:

  • it's very hard to test the mitmproxy as it is in goth since it's very closely bound to the Probe class, which in turn depends on the particular configuration of the Docker containers in the image, plus, it cooperates with an nginx server in those containers ...

the way forward as I see it:

  • extract the code that uses the mitmproxy and make its usage more straightforward so that it can be tested separately - possibly unit-tested
  • add tests that ensure that appropriate events are correctly captured from the mitmproxy before they are processed by the EventMonitor and before they're used in assertions

@shadeofblue shadeofblue removed their assignment Mar 14, 2023
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

2 participants