DanCardin
commented
2 months ago For sure, i'm interested in (at a minimum) any changeset that increases the overall coverage of the database object we already support!
What you have so far is pretty much most/all of what i think you'll need to do for the actual impl of the feature, modulo my PR comment.
As for tests, for most preexisting object types, it should be unnecessary to write any actual alembic test, so the easiest way to go about testing is generally just by copying any trigger existing test and just adapting to cover your usecase.
For this particular feature, i wouldn't think you'd need to worry about specific updating tests because any actual change to the arguments list will realistically require a replacement of the whole trigger. So i figure a trigger with a single argument, and a trigger with 2 arguments probably covers all the meaningful tests you could write.
As for documentation, i think just including a bit about how to use it in the docstring, as well as revising some of the notes in https://github.com/DanCardin/sqlalchemy-declarative-extensions/blob/main/docs/source/dialects/postgresql.md about how it only supports the audit feature is probably the sum total of what you'd need.
Finally, feel free to bump the minor version in the pyproject.toml, and include a note in the changelog. otherwise i'll do so after this gets merged anyway.
zimtkeks
coveralls
Hello,
first of all, I really appreciate this library and the work that went into it. It's a really useful addition when using an ORM and alembic.
There's one feature that is not supported, but relevant for my use-case, namely triggers that have arguments. Triggers without arguments are supported, though.
Being encouraged by your contribution guideline, I decided to implement changes, that extend
sqlalchemy_declarative_extensions.dialects.postgresql.trigger.Triggerand related code such that it works for my use-case. With some good will the present state may be considered as a PoC.Given that there is interest on maintainer-side for adding this feature, I'd like to extend this PR such that it is worthy of being merged (including test cases, documentation, etc.).
I'd like to contribute, but I also need guidance to do so. I'm fairly new to SQLA, postgres DBs and even contributing to an OSS project. I'm not sure how to get some feedback. So for now I hope a draft PR is fine for collecting and discussing code changes.
Please let me know if you're interested in a collaboration (also if you're not). I'll try to keep the workload low on your end. I'll probably only need a few hints. Maybe you have some already :-) (e.g. implementation issue I overlooked, some test that could serve as base to test the feature)
Thank you very much!