using oauth and openapi #86
Conversation
|
@fmigneault are you ok with this PR? The adapter (and maybe other things) need to be fixed. But this could happen in a follow-up PR. I have skipped the The xmlrpc interface is now replaced by an OpenAPI ... in the module Let me know if you like things changed are push them directly to this PR. |
There was a problem hiding this comment.
Multiple breaking changes for Magpie that should be somewhat easy to fix.
Also, many functionalities (and planned todos) duplicated from Magpie. I'm questioning why not use this component directly and combine efforts in improving it if there are any missing requirements for you?
twitcher/basicauth.py
Outdated
| authz_policy = ACLAuthorizationPolicy() | ||
| config.set_authorization_policy(authz_policy) | ||
| config.set_authentication_policy(authn_policy) | ||
| config.set_root_factory(lambda request: Root()) |
There was a problem hiding this comment.
This will need to be called via the adapter's configurator_factory as Magpie defines these with different settings.
There was a problem hiding this comment.
Only used by oauth2 module to register oauth clients. Made optional in config.
| @@ -53,6 +53,39 @@ Do the same as above using the ``Makefile``. | |||
| $ make lint | |||
| $ make coverage | |||
|
|
|||
| Upgrade Database | |||
| ---------------- | |||
There was a problem hiding this comment.
Maybe database migration should be done automatically, otherwise chances are Twitcher's schemas in the code vs the retrieved ones from the db will mismatch and just cause the app to crash.
Expected behaviour should be that the db and its schemas are migrated up/down on boot time to match the installed Twitcher version.
There was a problem hiding this comment.
Don't know ... I thought it would be safer to run make migrate intentionally. Though it might not harm when run each time before make start.
| This first step is only needed in development to generate a new database schema version. | ||
|
|
||
| Upgrade to that revision: | ||
| Before you can start the service you need to initialize or upgrade the database: |
|
@fmigneault Thanks for review :) I have made updates according to your comments. Please check if they fit, especially for the new I have removed the store interface ... the store is only used by the registry, so the registry interface should include it. |
twitcher/owsproxy.py
Outdated
| @@ -160,25 +159,11 @@ def owsproxy_view(request): | |||
| service = request.owsregistry.get_service_by_name(service_name) | |||
| except Exception: | |||
| return OWSAccessFailed("Could not find service") | |||
| if verify_request(request, service) is False: | |||
| if request.verify_ows_request is False: | |||
There was a problem hiding this comment.
naming is not intuitive as it feels like a function name
instead use is_ows_request_verified or similar
There was a problem hiding this comment.
done ... but shortened to is_verified.
|
@fmigneault ok to merge to master? |
@cehbrecht yes go ahead :) |
|
@fmigneault done. Thanks for you support :) |
|
@dbyrns @fmigneault There was a comment on the breaking changes in Twitcher introduced with version 0.6.x ... somehow I can not see it anymore. Looks like sins of the past. I hope it is not too much to adapt. I will add you in the review for future changes. You can also adapt the Twitcher. |
|
Thank you very much it's appreciated. Have a good day. |
This PR fixes issue #22.
Changes: