inveniosoftware / invenio-vocabularies

Invenio module for managing vocabularies.
https://invenio-vocabularies.readthedocs.io
MIT License
2 stars 40 forks source link

cli : Create factory for get_config_for_ds #298

Closed Samk13 closed 1 month ago

Samk13 commented 4 months ago

:heart: Thank you for your contribution!

Description

Address the FIXME noted in the following section: https://github.com/inveniosoftware/invenio-vocabularies/blob/a180d29f38f366b37d228314accac96fc74b6d81/invenio_vocabularies/cli.py#L27-L51

Preferably merge #297 first to pass the unrelated failing test.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.
Samk13 commented 4 months ago

Question: in datastreams/factories L40: https://github.com/inveniosoftware/invenio-vocabularies/blob/a180d29f38f366b37d228314accac96fc74b6d81/invenio_vocabularies/datastreams/factories.py#L28-L42 Specific factories like WriterFactory, ReaderFactory, and TransformerFactory inherit from both Factory and OptionsConfigMixin, accessing options() successfully. However, the base Factory class does not inherit from OptionsConfigMixin, leading to Pylint errors when accessing options() Class 'Factory' has no 'options' memberPylint.

Should we adjust the design to include OptionsConfigMixin in Factory for direct options() access, or is the separation intentional to restrict options() access to specific factories only?