jpassaro / audit-alembic Goto Github PK

Once https://gerrit.sqlalchemy.org/#/c/449/ is merged and released, undo the changes from ceee76d and set the required Alembic version accordingly.

Suppose you start out with the prefab table from Auditor.create(). Later you decide to bring it into your larger ORM fold and now you need to construct your Auditor using your existing table, not Auditor.create(). When running your first upgrade after this change, you want some kind of assurance that you got the table details right. For Alembic to issue a create_table could mean recreating the table, making a copy of it, creating a new column where you fat-fingered the old column name... Pain in the butt any way you imagine it. Better to refrain from creating it, and let SQLAlchemy alert you if something doesn't look right.

Create a better "home page" for the docs. Maybe single-page the actual content, just provide links for housekeeping stuff like changelog and authors.

Is the project still in-use and actively developed? It looks like it hasn't seen updates in a little while.
Thanks!

There is a restriction preventing this sort of thing:

with auditor1.setup():
    with auditor2.setup():  # raises an error
        ...

That is, if we're already inside one .setup() context, we block the user from creating another one.

Each .setup() context monkey-patches alembic.context.configure() with an on-the-fly function invoking alembic.context.configure() but adding that auditor as a listener. The normal usage is something like this, at the end of an env.py file:

with auditor.setup():
    if context.is_offline_mode():
        run_migrations_offline()
    else:
        run_migrations_online()

So why the restriction? The use case I foresaw for this was to track in only one table (which seems pretty obvious, at least until #1 is implemented). In that case the restriction certainly makes sense: obviously we do not want a single auditor added twice. I figured it was better just to error out than to risk a migration being logged twice.

If you do want to use multiple Auditors, there is a fairly simple solution, mentioned in the docstrings: lose .setup() and just use on_version_apply directly.

from alembic import context
context.configure(
  ...
  on_version_apply=[auditor1.listen, auditor2.listen],
  ...
)

I thought this was especially acceptable in light of .setup() being in fact a convenience method, allowing our library to be implemented quickly into an existing Alembic setup. It's okay for a convenience method to be less useful in a more complicated situation, no? Plus the above form is somewhat more transparent to the casual reader about what the library is actually providing you with; its only downside is that for most env.pys, it needs to be added twice.

This issue is to document the question, which I'm a little uncertain about: what to do about this. Is it fine as is? Is setup() sufficiently useful to remove this restriction? Or to the contrary, maybe the opacity of a .setup() context more harmful than the quick and easy patch it allows (after all, the alternative only slightly more verbose and a great deal more readable...); perhaps we should just get rid of setup() altogether?

I'm open to feedback here if anybody is using this library at all, or if you happen upon it and have experience enough with Alembic to opine.

We should provide template directories the way Flask-Migrate does. Helps new users (without existing Alembic environment) get a quick start, and helps Alembic users quickly see exactly how to integrate Auditor.

Docs should point in a very clear way to Alembic's excellent documentation about creating an environment that suits your needs...

Should also come with a CLI entry point that does the same as flask db init: namely invokes alembic init with the correct template.

Not sure if anybody would really need this, but it came out as a blind spot as I wrote the how to in the README. Can't hurt I suppose.