feat: add route_dependencies attribute to transaction extensions#885
Conversation
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'STAC FastAPI Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.
| Benchmark suite | Current: a3ff0fd | Previous: 2e2b0f9 | Ratio |
|---|---|---|---|
Items With Model validation (100) |
116.6396950108403 iter/sec (stddev: 0.006995198306056394) |
157.22439379368063 iter/sec (stddev: 0.00008602307183466387) |
1.35 |
This comment was automatically generated by workflow using github-action-benchmark.
06da220 to
1746daa
Compare
|
@jonhealy1 @vincentsarago this is ready for review. I think it will be an important feature for users to safely enable the transactions extension in downstream applications like stac-fastapi-pgstac/SFEOS. Users who want more fine-grained control could still use the per-route approach that existed before but now there is an easy button to turn on authentication for all transactions routes! |
jonhealy1
left a comment
There was a problem hiding this comment.
Nice work! This makes adding dependencies to the transaction routes a lot easier.
|
This same idea should be added to the catalogs transaction extension as well |
resolves #884
Related Issue(s):
Description:
This adds a new argument to the
TransactionExtensionandBulkTransactionExtensionthat makes it simple to add a dependency to all routes affected by the transactions extensions. It is possible that we would want to build off of the existingadd_route_dependenciespathway instead of doing this within the extension, see #884 for some discussion around that point.PR Checklist:
pre-commithooks pass locallymake test)make docs)