serokell / coffer

Multi-backend password store with multiple frontends
4 stars 2 forks source link

[#25] Creating one connection manager for all `BackendEffect` actions #54

Open DK318 opened 2 years ago

DK318 commented 2 years ago

Description

Problem

At this moment we are creating one connection manager per BackendEffect. This seems ridiculous.

Solution

Added Member (State (Maybe Manager)) constraint in Sem to store only one connection manager for all BackendEffect actions.

Related issue(s)

MagicRB commented 2 years ago

This is a bad idea IMO, having to create a tls connection manager for every backend is less then ideal. What we need is a way for each backend to store some data as a somehow

dcastro commented 2 years ago

having to create a tls connection manager for every backend is less then ideal

How so?

What we need is a way for each backend to store some data as a somehow

I didn't really understand what you meant here. What do you mean? And how is this related to connection managers?

MagicRB commented 2 years ago

My new pass backend doesnt need a tls connection mamager, so adding a Reader Tls... as a dependency for all backends is not ideal https://github.com/serokell/coffer/pull/54/files#diff-a0dd0bee522a338924cfebf12a6c79b6cb8a9bfdd093e249d370306060b45480R32

MagicRB commented 2 years ago

We're once again making the frontend take responsibility for things which should not be its responsibility, also such state will make it hard for #56 to work.

dcastro commented 2 years ago

@MagicRB Oh I see, I hadn't considered that some backends might not need a connection manager at all.

Yup, that makes sense. Would you please open a PR that solves the issue in #16?

MagicRB commented 2 years ago

I think I've come up with a way to do this, I'll get it done today