Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: remove dependencies on unnecessary Invenio-* packages #134

Open
1 task
jirikuncar opened this issue Nov 7, 2016 · 5 comments
Open
1 task

tests: remove dependencies on unnecessary Invenio-* packages #134

jirikuncar opened this issue Nov 7, 2016 · 5 comments

Comments

@jirikuncar
Copy link
Member

jirikuncar commented Nov 7, 2016

It would make package release process easier.

@jirikuncar jirikuncar added this to the I3B-API-Docs milestone Nov 7, 2016
@nharraud
Copy link
Member

nharraud commented Nov 9, 2016

I don't see any issue with that.
It seems that the only change which this would require would be to change the config:

RECORDS_UI_ENDPOINTS = dict(
            recid=dict(
                # ...
                route='/records/<pid_value/files/<filename>',
                view_imp='invenio_files_rest.views.file_download_ui',
                record_class='invenio_records_files.api:Record',
            )
        )

Changed with

- view_imp='invenio_files_rest.views.file_download_ui',
+ view_imp='invenio_records_files.views.file_download_ui',

Do we need to keep record_file_factory at all? It seems that it is only used by file_download_ui.

In case somebody wants to use a different record_file_factory they can at the same time provide their own file UI endpoint as it means that there is probably not a 1 record -> 1 bucket relation anymore.

@lnielsen
Copy link
Member

lnielsen commented Nov 9, 2016

👍

Would make Files-REST more clean. @nharraud I would still keep record_file_factory (which is anyway already located in Records-Files).

@nharraud
Copy link
Member

nharraud commented Nov 9, 2016

@lnielsen I meant removing it from the ext.py file in invenio-files-rest, not removing the function which is in invenio-records-files. If we move file_download_ui to invenio-records-files it can directly use the function instead of using a config variable.

jirikuncar added a commit to jirikuncar/invenio-files-rest that referenced this issue Nov 9, 2016
* Removes `record_file_factory` extenstion property. (addresses inveniosoftware#134)

* Moves `file_download_ui` utility function to Invenio-Records-Files.

Signed-off-by: Jiri Kuncar <[email protected]>
jirikuncar added a commit to jirikuncar/invenio-files-rest that referenced this issue Nov 9, 2016
* Removes `record_file_factory` extenstion property. (addresses inveniosoftware#134)

* Moves `file_download_ui` utility function to Invenio-Records-Files.

Signed-off-by: Jiri Kuncar <[email protected]>
jirikuncar added a commit to jirikuncar/invenio-files-rest that referenced this issue Nov 9, 2016
* Removes `record_file_factory` extenstion property. (addresses inveniosoftware#134)

* Moves `file_download_ui` utility function to Invenio-Records-Files.

* NOTE One must fix import paths in `RECORDS_UI_ENDPOINTS`.

Signed-off-by: Jiri Kuncar <[email protected]>
@jirikuncar
Copy link
Member Author

  • invenio-access - used in permissions.py - named tuple + flask_security.Permission when Invenio-Access is not installed
  • invenio-accounts easy to remove after Invenio-Access is out

@lnielsen lnielsen modified the milestones: I3B-API-Docs, someday Feb 19, 2019
@topless
Copy link
Member

topless commented May 22, 2019

@lnielsen after our discussion I believe this one can be closed, the removal of invenio-access is not easy or feasible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants