Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

Restructures the code base with more object methods #55

Merged
merged 3 commits into from
Jan 10, 2020
Merged

Conversation

kushaldas
Copy link
Contributor

@kushaldas kushaldas commented Jan 6, 2020

Moves configuration related code and also callbacks as proper methods into Proxy class.

proxy.py

Now, Proxy class gets a must conf_path argument, and it creates the inital
`conf` attribute from that.  Proxy also has default on_save and on_done and
err_on_done methods.  def handle_response(self) method has a `assert
self.res` to mark that res is populated before this call. This is for mypy

main.py

Mostly has changes from black

entrypoint.py

No need for fancy dynamic err_call_back, the proxy object will call
self.err_call_back if any issue in reading configuration.

The test cases now have their own configuration files to create the proxy
object. Also, to do proper dynamic attachment of any method of Proxy class
we are using https://docs.python.org/3/library/types.html#types.MethodType
so that our own on_save or on_done or err_on_done will be called during
tests.

stretch test will fail as it does not have any variable level type annotation.

Testing

  1. Build the securedrop-proxy source distribution:
    • check out this branch
    • run python setup.py sdist
  2. Build the .deb:
    • in a clone of securedrop-debian-packaging, run PKG_PATH=/home/user/src/securedrop-proxy/dist/securedrop-proxy-0.1.6.tar.gz PKG_VERSION=0.1.6 make securedrop-proxy (adjust the path to your working copy of this repo as necessary)
  3. Install the .deb in sd-proxy-buster-template:
    • qvm-copy ~/debbuild/packaging/securedrop-proxy_0.1.6+buster_all.deb and specify sd-proxy-buster-template as the target VM
    • In a shell on sd-proxy-buster-template, run:
      • cp ~/QubesIncoming/sd-dev/securedrop-proxy_0.1.6+buster_all.deb /tmp
      • sudo apt install /tmp/securedrop-proxy_0.1.6+buster_all.deb
  4. Reboot.
  5. Start the SecureDrop client in sd-svs and confirm that the proxy is communicating with the server: submissions and replies should populate with no errors.

@kushaldas kushaldas changed the title Adds type annotation + black reformatting [WIP] Adds type annotation + black reformatting Jan 6, 2020
@kushaldas kushaldas force-pushed the more_typing branch 4 times, most recently from df2dc21 to 20d029e Compare January 7, 2020 10:58
proxy.py

    Now, Proxy class gets a must conf_path argument, and it creates the inital
    `conf` attribute from that.  Proxy also has default on_save and on_done and
    err_on_done methods.  def handle_response(self) method has a `assert
    self.res` to mark that res is populated before this call. This is for mypy

main.py
    Mostly has changes from black

entrypoint.py
    No need for fancy dynamic err_call_back, the proxy object will call
    self.err_call_back if any issue in reading configuration.

The test cases now have their own configuration files to create the proxy
object. Also, to do proper dynamic attachment of any method of Proxy class
we are using https://docs.python.org/3/library/types.html#types.MethodType
so that our own on_save or on_done or err_on_done will be called during
tests.
@kushaldas kushaldas changed the title [WIP] Adds type annotation + black reformatting Restructures the code base with more object methods Jan 7, 2020
redshiftzero added a commit to freedomofpress/securedrop-builder that referenced this pull request Jan 7, 2020
@redshiftzero
Copy link
Contributor

note that we should merge freedomofpress/securedrop-builder#120 first otherwise we'll break stretch nightlies

@redshiftzero
Copy link
Contributor

this looks great! diff lgtm here but I noticed that the test plan involved installing the deb to sd-svs whereas we want it in sd-proxy, I've updated it - can you confirm you tested on sd-proxy? whichever of you @rmol or @kushaldas confirms that just note in a comment on this PR

@kushaldas
Copy link
Contributor Author

I copy pasted in sd-proxy and tested.

redshiftzero
redshiftzero previously approved these changes Jan 9, 2020
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

diff lgtm, and gave this in a spin in Qubes, all good - thanks for these changes!

@rmol I'll wait before merging so you have a chance to comment on this

Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

Looks good, except for one preexisting problem your changes made me notice, and a check that's obsoleted by your work. I did not test in Qubes since @redshiftzero already did.

securedrop_proxy/entrypoint.py Show resolved Hide resolved
securedrop_proxy/proxy.py Outdated Show resolved Hide resolved
@rmol rmol merged commit 21c2963 into master Jan 10, 2020
@rmol rmol deleted the more_typing branch January 10, 2020 14:44
@rmol rmol mentioned this pull request Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants