cosmicpython / code

Example application code for the python architecture book
Other
2.07k stars 926 forks source link

move start_mapping() out of flask app #5

Closed daniel-butler closed 4 years ago

daniel-butler commented 4 years ago

Should the orm.start_mappers() be moved to the repository or unit of work? It seems out of place in the flask api.

I'm learning towards the repository because it is the connection between the database and the model. What are your thoughts?

daniel-butler commented 4 years ago

Maybe it makes more sense in the unit of work that way its not called every time uow is used with a context manager. I'm thinking something like this:

class SqlAlchemyUnitOfWork(AbstractUnitOfWork):

    def __init__(self, session_factory=DEFAULT_SESSION_FACTORY):
        self.session_factory = session_factory
        orm.start_mappers()  # <--------------------------------------- here

    def __enter__(self):
        self.session = self.session_factory()
        self.init_repositories(
            batches=repository.SqlAlchemyRepository(self.session),  # <-- in here it would be called each time the context manager is used
        )
        return super().__enter__()

    def __exit__(self, *args):
hjwp commented 4 years ago

Good question!

start_mapping() is an example of the kind of thing we'd usually put in some sort of "bootstrap" code: https://github.com/python-leap/book/blob/master/appendix_bootstrap.asciidoc

I can see why you'd want to put it with the repository or with the uow maybe, but the problem is that start_mappers() needs to be called exactly once, and you're likely to instantiate several repositories and uows over the lifecycle of your app process...

daniel-butler commented 4 years ago

Makes sense. I haven’t read that part yet

daniel-butler commented 4 years ago

Is bootstrap implemented in the code? I couldn't find it when searching

hjwp commented 4 years ago

did you read the appendix? https://github.com/cosmicpython/book/blob/master/appendix_bootstrap.asciidoc#bootstrap-script