Stack.refresh doesn't use original get_by_id kwargs

[Steve Baker]

This will lose args such as tenant_safe and eager_load which will cause problems in some scenarios.

[Zane Bitter (zaneb)]

Ugh, I didn't realise that the versioned objects interface had its own refresh() method.

How SQLAlchemy is supposed to work is that it caches the db object in a session. When you need to look up the object again by ID, you just do get() and it returns the cached one without having to do another query on the database. It also does nice things like invalidate the data after a transaction, so that it will automatically be refetched from the database with a query when it is required, and not when it is not. You can, of course, also invalidate or refresh the data manually whenever you want.

We already threw out the performance part ages ago by needlessly doing a query when looking up anything, even when we have its ID already:

https://review.openstack.org/#/c/38817/6/heat/db/sqlalchemy/api.py

We had to hack in a parameter to every load() API to pass the db object just to keep things marginally performant.

Now with versioned objects we have data that is completely disconnected from its underlying SQLAlchemy object, so we no longer have any safeguards around it becoming stale. Not to mention the 50 obscure parameters with weird security implications that you have to specify every time just to get the latest version of some data you already have.

We need a serious rewrite of the DB code. We have broken abstractions on top of broken abstractions and hacks on tops of hacks at this point. Stacks, StackLocks and the services code are all intertwined with one another in a tangled mess of spaghetti that makes it next to impossible to figure out when (or even if) any given piece of data will get written. It's worse than before I implemented load() and store() for all the classes that have DB representations.