encore
encore copied to clipboard
Add option for avoiding using `Named` for getting the driver for the current system's DB
See Slack thread: https://encoredev.slack.com/archives/CQFNUESN9/p1664736264406129
I'll start on that one for Hacktoberfest
I'm not convinced this is a good idea. I'm worried it increases the risk of deleting or modifying data you didn't intend to, because a code snippet was moved from one service to another.
What's the pain point you were feeling that motivated your wanting this, @Minivera?
The pain point comes from using a third-party ORM or another tool for connecting to a database within a service, for example in this guide: https://encore.dev/docs/how-to/entgo-orm. I've had to refactor some code between services on some instances and I've had multiple errors because I forgot to rename the Named parameter, which can be tough to debug when you don't expect it. Named also seems to not be checking for the database's existence (? not sure), so errors are tough to catch. For me, the worst ones are when I rename a system's folder (Example, I want to rename usr to user, I have to find the Named calls and make sure to also rename them, otherwise it fails silently. Always connecting to the current db would solve that.)
From my limited perspective, this risk seems to be the same for both methods? (Or even higher using Named)
- If using
Namedfollowed byStdlib, you'll connect to that database. If a file is moved from one system to another, that connection still happens even if you moved the migrations around. It would seem like that is still at a risk of deleting data or running into errors if you changed the database you connected to, but forgot to change the name inNamed. Let's say for example I moved a file from theuserto theproductsystem, but forgot to rename theNamedand left someusercode in there, theproductsystem might modify theuserdb without me realizing it? - If always connecting to the current service's DB. If you move a file to another system, that connection will also change which db it connects to. If you also moved the migration files, then most of the "new" tables should be empty? If not, the likelihood of a table being exactly the same is fairly low, so the risk is more about running into SQL errors telling you that the SQL queries you're trying to run are not valid, and less about you running into the risk of deleting stuff.
I'm still confusing myself with the system v.s. service terminology, so hopefully this helps.
In my opinion, controlling and creating databases through code would be better, but this small change could also help a lot in the meantime.
I think introducing sqldb.NewDatabase("name") and have that return a *sqldb.DB would solve a lot of these issues, and give you more control over database creation and decouple the name of the database from the name of the service the code lives in. And if the compiler doesn't catch the issue of sqldb.Named("blah") referencing a database that doesn't exist, we should fix that. Would those two changes combine address the issues you're describing?
That would yes! Do you think the first one (sqldb.NewDatabase("name")) would be something worth investigating during hacktoberfest? I can work on the second one no problem, but the first one might be more complex (Happy to start with a proposal there)
I created the issue for the sqldb.NewDatabase("name") proposal, feel free to edit it or close it if that's not what you have in mind. I had some thoughts and I like to throw things down in written form before I forget.