alex / alchimia

MIT License
102 stars 22 forks source link

Autoloading tables #19

Closed taion closed 7 years ago

taion commented 9 years ago

Currently, autoloading tables doesn't really work. If I try to do:

table = Table(table_name, metadata, autoload=True)

The call fails because Table._autoload() tries to use Engine.bind_callable(), which isn't defined on TwistedEngine. However, you can do the following:

table = Table(table_name, metadata, autoload=True, autoload_with=engine._engine)

However, it's a bit messy to try to access engine._engine. Would it make sense to either override Engine.bind_callable() or to expose a more formal way of accessing the underlying engine with a property or something?

I believe it's fine to make the blocking call for this use case, since you should only be reflecting on the tables at startup time.

alex commented 9 years ago

Are you sure bind_callable is the right method? I can't find any references to that in the sqlalchemy source code.

taion commented 9 years ago

I'm in fact sure that it's not. I meant run_callable. Sorry.

alex commented 9 years ago

run_callable on Connection would be easy enough to implement, but I'm skeptical of adding it to TwistedEngine, since if someone accidentally used it, everything would block and they wouldn't know why. I think autoload_with is your best bet for now.

taion commented 9 years ago

And the only place it's used in SQLAlchemy proper is for autoload, but it is actually a documented feature, though I'm not sure it's something particularly useful to a SQLAlchemy user. Agree that it could potentially be dangerous if someone were to use it for some reason, though.

What do you think of adding something like

@property
def sync_engine(self):
    return self._engine

I just feel a little dirty using engine._engine in my code.

alex commented 9 years ago

Hmm, sync_engine is an interesting proposal. I think it might be better if users just created their own second engine though -- that way stuff is more explicit, and it's not really any more work.

On Fri Nov 21 2014 at 3:22:17 PM Jimmy Jia notifications@github.com wrote:

And the only place it's used in SQLAlchemy proper is for autoload, but it is actually a documented feature, though I'm not sure it's something particularly useful to a SQLAlchemy user. Agree that it could potentially be dangerous if someone were to use it for some reason, though.

What do you think of adding something like

@propertydef sync_engine(self): return self._engine

I just feel a little dirty using engine._engine in my code.

— Reply to this email directly or view it on GitHub https://github.com/alex/alchimia/issues/19#issuecomment-64053778.

taion commented 9 years ago

Doesn't that require creating a second connection pool? I guess I'd have to tear down the second engine afterward?

alex commented 9 years ago

Well, it sounds like the main use case for this is to use it once-off to do stuff like create tables, not use it as part of a long-running application, right?

If that's the use case, creating a 2nd connection pool doesn't really matter. If there's a use case for this as a part of a long-running process, I'll have to think more.

On Sat Nov 22 2014 at 4:03:19 PM Jimmy Jia notifications@github.com wrote:

Doesn't that require creating a second connection pool? I guess I'd have to tear down the second engine afterward?

— Reply to this email directly or view it on GitHub https://github.com/alex/alchimia/issues/19#issuecomment-64100417.

taion commented 9 years ago

The use case is to introspect table definitions at start up time. The app will continue to run indefinitely after that.

alex commented 9 years ago

making a 2nd throw-away connection pool won't really effect the performance of that use case, as far as I can tell

On Sat Nov 22 2014 at 4:06:24 PM Jimmy Jia notifications@github.com wrote:

The use case is to introspect table definitions at start up time. The app will continue to run indefinitely after that.

— Reply to this email directly or view it on GitHub https://github.com/alex/alchimia/issues/19#issuecomment-64100495.

taion commented 9 years ago

Assuming epoll/kqueue/whatever, sure. Is there a concrete downside to properly exposing the underlying non-async engine, though? I understand that it's not a generally useful thing, but it seems like it could nicely support a bunch of edge cases like this. Introspecting tables in particular seems like a pretty common pattern to me.

taion commented 7 years ago

Closing for staleness. In retrospect, using engine._engine isn't that bad. And really I should have been using the ORM in the first place.