olirice / alembic_utils

An alembic/sqlalchemy extension for migrating sql views, functions, triggers, and policies
MIT License
193 stars 42 forks source link

Optimize solve_resolution_order a bit #90

Closed nstrong-scw closed 2 years ago

nstrong-scw commented 2 years ago

== DETAILS This doesn't change the strategy very much, but a couple optimizations:

I had difficulty running the tests. Anything that invoked alembic gets a KeyError: 'formatter' error, which seems to indicate it's not loading alembic.ini correctly, but I'm not sure how to solve that.

However, I did do a basic smoke test with my own sqlalchemy/alembic/alembic_utils project.

olirice commented 2 years ago

You're right that it is unnecessary to loop over entities that have already been resolved, but they are skipped immediately

The cost of checking inclusion in a list is negligible compared to

with simulate_entity(sess, entity, dependencies=resolved):

so I wouldn't expect measurable performance gains from the change and it introduces additional variables to track so I'd prefer not to add the complexity

olirice commented 2 years ago

if you'd like to open another PR to move the log statement I'm game for that!