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

api: resolve Record API subclassing limits #17

Open
nharraud opened this issue Jun 10, 2016 · 4 comments
Open

api: resolve Record API subclassing limits #17

nharraud opened this issue Jun 10, 2016 · 4 comments

Comments

@nharraud
Copy link
Member

nharraud commented Jun 10, 2016

The class invenio_files_records.api.Record adds a feature to invenio_records.api.Record which is the RecordsBuckets deletion when Record.delete(...) is called.

Question 1:
What do we do for Invenio module generic enough to use directly the invenio_records.api.Record? If they call invenio_records.api.Record.delete(...) the bucket won't be deleted.
On the other hand if we make every module use invenio_files_records.api.Record then they can't work without invenio_records.api.Record. Thus Tind which has its own files module won't be able to use many Invenio modules.
We can make the class configurable as it is now in Deposit. But then it should become a pattern which we use in every module.

Question 2:
Subclasses of invenio_records.api.Record.get_record() will return any matching record. It is not overriden by invenio_records.api.Record.get_record(). Thus it is possible to get a non invenio_records.api.Record with invenio_records.api.Record.get_record() and calling delete on it will fail as it does not have any bucket.

cc @remileduc @PXke

What do you think @jirikuncar @lnielsen?

@nharraud
Copy link
Member Author

nharraud commented Jun 10, 2016

A few questions regarding Deposit's Record API.

  • Do we need it to BE a Record or to BEHAVE like a record?
  • If we need it to BE a Record. Do we need it to be in the same table as the published records?
  • Are there needs for differentiating in the database different kind of records like Deposit and published Records? (example different sharding).

cc @tiborsimko

@nharraud
Copy link
Member Author

It seems to me that we only need to differentiate the type of the record, i.e: published record, deposit and we need to be able to override the API class for any of them. Example: invenio_records_files.api.Record would probably replace in most case invenio_records.api.Record.

We have some modules which need to have access to all types of Records (ex: invenio_indexer)

We have some modules which need to make a difference between types of Records (ex: invenio_communities for accepting or refusing published records, it does not accept deposits).

Please don't hesitate to correct me if I am making wrong assumptions.

@nharraud
Copy link
Member Author

As there is nothing to differentiate the types of records, all the invenio record-indexers I have seen up to now use the elasticsearch index name to differentiate them. This means that there is no way to index two types of records in the same index (as parent-child document for example).

As this is an API design issue, IMHO it needs to be resolved before the Invenio 3 beta release.

cc @inveniosoftware/triagers

@jirikuncar
Copy link
Member

I'm sorry, but I don't really get the problem. Each module provides an API and other may or may not use it. If an overlay doesn't want to have support for files, it is going to use invenio_records.api.Record otherwise invenio_records_files.api.Record. Other Invenio packages can be opinionated and require specific API or provide configuration so somebody can supply its own class.

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

2 participants