global: records permissions dependency removal #53
Conversation
| @@ -0,0 +1,42 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
Isn't this just pushing the problem from inveniosoftware/invenio-records#137 down the stack? I.e. I don't think it should be added here.
For the example app, i think you can create an example of a permission factory that relies on metadata inside the record.
There was a problem hiding this comment.
Which permission factory do you propose as default for RECORDS_UI_DEFAULT_PERMISSION_FACTORY?
There was a problem hiding this comment.
It means that I will remove also the tests that involve permissions?
There was a problem hiding this comment.
deny_all by default? update: maybe too restrictive ...
There was a problem hiding this comment.
None looks fine because of this if
There was a problem hiding this comment.
@lnielsen @jirikuncar
what would you like to do with the docs?
Could you help me to change it (if needed)?
There was a problem hiding this comment.
IMHO we can remove the reference to invenio_records and replace it with perm_factory defined in the paragraph above.
|
@jirikuncar @lnielsen |
|
@hachreak In my opinion not, because it means something is wrong in the system. You have a record which is not deleted, not redirected etc, on which you have setup and endpoint, but it can't resolve down to a record. |
hachreak
force-pushed
the
138_permission
branch
2 times, most recently
from
d480104
to
fa7939b
Compare
| @@ -268,7 +268,7 @@ | |||
| permission factory: | |||
|
|
|||
| >>> app.config['RECORDS_UI_DEFAULT_PERMISSION_FACTORY'] = \ | |||
| ... 'invenio_records.permissions:permission_factory' | |||
| ... 'mymodule:my_permission_factory' | |||
There was a problem hiding this comment.
What about using perm_factory directly here instead a fictive path?
hachreak
force-pushed
the
138_permission
branch
from
fa7939b
to
308b6dc
Compare
| @@ -48,7 +47,8 @@ def permission_factory(self): | |||
| """Load default permission factory.""" | |||
| if self._permission_factory is None: | |||
| imp = self.app.config['RECORDS_UI_DEFAULT_PERMISSION_FACTORY'] | |||
| self._permission_factory = import_string(imp) if imp else None | |||
| self._permission_factory = obj_or_import_string(imp) \ | |||
| if imp else None | |||
hachreak
force-pushed
the
138_permission
branch
from
308b6dc
to
27e81f2
Compare
* Removes invenio-records permissions dependency. (addresses inveniosoftware/invenio-records#138) Signed-off-by: Leonardo Rossi <[email protected]>
hachreak
force-pushed
the
138_permission
branch
from
27e81f2
to
6a6f26c
Compare
|
ping @jirikuncar |
(addresses global: removal of access permissions invenio-records#138)
Signed-off-by: Leonardo Rossi [email protected]