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

Remove ABC, parameterize database access, and remove locks #13

Closed
wants to merge 13 commits into from
Closed

Remove ABC, parameterize database access, and remove locks #13

wants to merge 13 commits into from

Conversation

richpsharp
Copy link
Collaborator

This PR:

  • removes the abstract helper class that was supposed to aid in iterative development when the code base of a module/class/function was unreadable. In practice that turned out to be a bad design. closes Remove abstract class abstraction #12
  • adds a function that isolates accesses to the sqlite3 database used by TaskGraph. This makes the code base cleaner and also makes it possible for multiple TaskGraph objects to exist in parallel and use the same database. Note it does not guard against identical operations being executed in parallel, but anything else is fine. closes Support multiple taskgraph objects in parallel #11
  • All the locks guarding access to the TaskGraph object global state have been removed. These only existed because I thought I needed to guard against reading while writing to global state. The GIL makes all those actions atomic and hence unnecessary. That's why this PR has so many line changes in Task. A lot of them are indenting one to the left. closes Remove unnecessary locks on taskgraph #10

@richpsharp richpsharp added this to the 0.9 release milestone Mar 1, 2020
@richpsharp richpsharp self-assigned this Mar 1, 2020
@richpsharp
Copy link
Collaborator Author

@dcdenu4 , I added you as a reviewer here in case you'd find it helpful to get another critical look at the TaskGraph code base. James mentioned you might be programming with it on your next project. Feel free to decline though for whatever reason, I don't mean to force you to do it!

@richpsharp richpsharp closed this Mar 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant