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

Composition instead of inheritance for PostgresJobStore #142

Closed
ewjoachim opened this issue Feb 7, 2020 · 0 comments · Fixed by #147
Closed

Composition instead of inheritance for PostgresJobStore #142

ewjoachim opened this issue Feb 7, 2020 · 0 comments · Fixed by #147
Assignees
Labels
Issue appropriate for: People up for a challenge 🤯 This issue probably will be challenging to tackle Issue contains: Exploration & Design decisions 🧩 We don't know how this will be implemented yet Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code

Comments

@ewjoachim
Copy link
Member

ewjoachim commented Feb 7, 2020

We're dealing with fine software design issues, it's really no big deal if we don't go this way.

I think we could make a last (haha, last...) modification on how our job store handles being plugged with a real Postgres database versus being in memory for tests and such . Today, it's done via inheritance, with a BaseJobStore, a PostgresJobStore and an ImMemoryJobStore. I've finally realized that it would be much cleaner in terms of separation of concerns if the whole idea of store was uncorrelated from the idea of DB connection. We could have a JobStore, and a PostgresConnector and an InMemoryConnector, and the JobStore would receive an ...Connector instance at creation time (same way as the App currently receives a Job Store instance).

That would mean the App would be given a connector instead of a JobStore. If we change the name, that would be backwards incompatible, but we could still keep the name PostgresJobStore as an alias of PostgresConnector for at least a few releases.

@ewjoachim ewjoachim assigned ewjoachim and unassigned ewjoachim Feb 7, 2020
@ewjoachim ewjoachim added Issue contains: Exploration & Design decisions 🧩 We don't know how this will be implemented yet Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code Issue appropriate for: People up for a challenge 🤯 This issue probably will be challenging to tackle Type: Refactor labels Feb 7, 2020
@ewjoachim ewjoachim mentioned this issue Feb 8, 2020
3 tasks
@ewjoachim ewjoachim self-assigned this Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue appropriate for: People up for a challenge 🤯 This issue probably will be challenging to tackle Issue contains: Exploration & Design decisions 🧩 We don't know how this will be implemented yet Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant