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

bugfix when loading plugins #92

Merged
merged 1 commit into from
Sep 26, 2019

Conversation

normal-cock
Copy link
Contributor

The reason why except HttpProtocolException as e doesn't work as expected is that the way we load plugin in proxy.py is improper. We should not import a module itself in it, instead try to use importlib.import_module("__main__").

For example, suppose we have a test2.py the content of which is as following. The result of the code is False, and that's why except HttpProtocolException as e doesn't work.

# test2.py
class A:
    pass

from test2 import A as B

if __name__ == "__main__":
    print(A==B)
    exit(0)

@abhinavsingh
Copy link
Owner

@normal-cock Awesome. Makes sense, this is much better. But why have you deleted python-package.yml in the PR?

proxy.py Outdated Show resolved Hide resolved
proxy.py Outdated Show resolved Hide resolved
proxy.py Outdated Show resolved Hide resolved
@normal-cock
Copy link
Contributor Author

@normal-cock Awesome. Makes sense, this is much better. But why have you deleted python-package.yml in the PR?

@abhinavsingh I remove python-package.yml because I don't think it should be a part of the project itself. Most importantly, the file makes me unable to push a new branch to my own github repository :( . I think it's better to delete python-package.yml in repository and keep it local by adding it in .gitignore. Wdyt?

@abhinavsingh
Copy link
Owner

@normal-cock python-package.yml belongs to GitHub CI/CD integration, it's similar to .travis.yml file in the repository. python-package.yml is part of GitHub actions which results in CI/CD run for every commit and PR. See https://github.com/abhinavsingh/proxy.py/actions

I am not sure but do you have to enable actions for your account? GitHub actions is relatively new and unsure if it's available for all, but no reason why you can't check into your repository. What errors do you get when you try to commit to your repo?

@normal-cock
Copy link
Contributor Author

normal-cock commented Sep 26, 2019

@abhinavsingh The error message when I keep python-package.yml is that:

! [remote rejected] develop2 -> develop2 (refusing to allow an integration to create or update .github/workflows/python-package.yml)

@abhinavsingh
Copy link
Owner

@abhinavsingh The error message when I keep python-package.yml is that:

! [remote rejected] develop2 -> develop2 (refusing to allow an integration to create or update .github/workflows/python-package.yml)

Signup for actions here https://github.com/features/actions and try again. Github actions is a new feature from Github, its surprising that they are refusing you to push into repo. My guess is since you don't have actions enabled, you are unable to push.

Can you try and let me know. Thanks

@normal-cock
Copy link
Contributor Author

I've enable actions and the error is still there. @abhinavsingh

@abhinavsingh
Copy link
Owner

Reading the error message again refusing to allow an integration to create or update, looks like you cannot create or update existing Github action, unsure how you ended up deleting it in first place then :)

My guess is, start in a new branch which still contains .github/workflows folder, un-edited. Then you should be able to push your edits for other files. Can you try and confirm? Will try to also raise this with Github, doesn't sound right to me.

@normal-cock
Copy link
Contributor Author

normal-cock commented Sep 26, 2019

start in a new branch which still contains .github/workflows folder, un-edited. Then you should be able to push your edits for other files.

@abhinavsingh The error is unchanged.

The phenomena is that only if I delete .github/workflows in git, can I push new branch to my repository. That's strange.

@normal-cock
Copy link
Contributor Author

@abhinavsingh
Copy link
Owner

Thanks for the find. From that thread:

Is there any solution to this? Forcing the contributors to use SSH is not an option.

Did you close using git clone https://github.com/abhinavsingh/proxy.py.git or git clone git@github.com:abhinavsingh/proxy.py.git. Does the later one help you to push? This is indeed annoying from Github.

@normal-cock
Copy link
Contributor Author

@abhinavsingh Cool, it works after I change my origin from https://github.com/normal-cock/proxy.py.git to git@github.com:normal-cock/proxy.py.git

@normal-cock
Copy link
Contributor Author

I've change the pr. @abhinavsingh

@abhinavsingh
Copy link
Owner

I've change the pr. @abhinavsingh

Awesome, can you make sure tests are passing. I haven't used importlib.import_module('__main__') myself before and want to make sure it works as expected. Thank you.

.travis.yml Outdated Show resolved Hide resolved
proxy.py Show resolved Hide resolved
@abhinavsingh
Copy link
Owner

Kindly Squash and merge. Thank you. LGTM.

proxy.py Show resolved Hide resolved
@abhinavsingh abhinavsingh merged commit 3a5c780 into abhinavsingh:develop Sep 26, 2019
abhinavsingh added a commit that referenced this pull request Oct 10, 2019
* Initialize skeleton electron app

* Attempt to open devtools

* Electron free

* Initialize public/devtools

* Add basic support for static file serving and chrome devtools.

1. No cache header management for static file serving yet.
2. No chunked encoded responses for static files yet.
3. Chrome Devtool initialization.

* Fix static serving with query params

* profile using py-spy

* Complete websocket client loop

* lint check

* Add support for building websocket frames

* Remove redundant CDT params

* Lint check

* Refactor web server base plugin name

* Devtools integrated, need more polish

* Add START_TIME global var

* lint fix

* Remove outdated chrome rdp

* Add FAQs

* Add FAQs

* socket_connection decorator + context manager

* Defer SSL handshake and plugin initialize until protocol handler thread
has started.

This is a follow up to this PR
#111

* Add tests for new_socket_connection and its friend socket_connection

* Address an issue which came back after being fixed in #92

* Lint fixes

* uff ye str and bytes

* Remove explicit flushes outside of write ready descriptor handlers

* add links to import proxy

* Only try websocket upgrade if a route is registered

* Add plugin_examples.WebServerPlugin and use precision logging for levelname

* Remove redundant comments

* Add --devtools-ws-path flag

* Add on_websocket_open and on_websocket_close callbacks

* Add empty stubs for incomplete CDT responses

* Ensure client is ready before final flush

* Shutdown on write side of socket, may be client is still reading

* Since client.closed can be set, explicitly call client.connection.closed

* Add ModifyPostDataPlugin example.

Was first asked and referenced here
#115

* Start adding TestHttpProxyPlugin

* Fixes #116
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

Successfully merging this pull request may close these issues.

None yet

2 participants