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

Use gloabl session maker #395

Closed
wants to merge 3 commits into from
Closed

Use gloabl session maker #395

wants to merge 3 commits into from

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Jun 1, 2019

Description

Resolves issue #392

  • Removes make_session_maker and now create a global session factory called Session in db.py and configure it in app.py.

  • Controller no longer manages the application session; it not longer has a sessionmaker or a session that is accessed by widgets. We no longer need to pass around Sessions in order to share a session across separate sections of code. If we call the global session factory twice we get back the same session.

The following changes were also added while working to remove all references to the Controller's session object:

  • Removes source_exists and its one reference in widgets.py::on_source_changed since it is unnecessary when we already check if the source widget exists (it will not exists if the source was deleted from the db). This is relevant since this was one of the areas of code that referenced controller.session. Instead of rewriting it and then removing it later, I decided to just remove it.

  • Removes controller.get_file and reference to it in FileWidget constructor since we are now initializing FileWidget with a File object.

  • Moves database operation for adding reply from logic.py to storage.py

  • Models that have a relationship to other models now pass lazy='subquery' when defining their relationships in order to do subquery loading, see https://docs.sqlalchemy.org/en/13/orm/loading_relationships.html#relationship-loading-techniques. This gets used when we pass source.collection to update_conversation

Test Plan

  1. Send a file via the Source Interface and download it, see it change to "Open" status
  2. Open the file and check the logs for debug "Opening file..." statement
  3. Send a reply, refresh, see it remain
  4. Run through relevant https://github.com/freedomofpress/securedrop-client/wiki/Test-plan checks since this touches a lot of code

@sssoleileraaa
Copy link
Contributor Author

i still need to fix one test

@sssoleileraaa
Copy link
Contributor Author

Will reopen after modifying code to use contextmanager

@sssoleileraaa sssoleileraaa deleted the refactor-session branch May 26, 2020 18:00
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.

1 participant