-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add reindex
method to MNAdmin API
#1716
Comments
This is good, Jing. I suggest it should be a single Admin APIAdminister repository deployments. The admin API methods are used to administer a repository deployment, and the API MUST be restricted to authenticated subjects with administrative permissions. While the API overall may have utility across the DataONE network, the implementation of these methods will vary across different software implementations of the DataONE API, and some implementations may not need all (or any) of these facilities. The Admin service and all of its API methods are therefore optional and can be omitted from implementation, or may return
|
reindex
and reindexall
methods to Metacat Admin APIreindex
method to MNAdmin API
Looks good! I like the idea to use an explicit parameter to indicate if Metacat indexes all pids. This can avoid accidental time-consuming processes. I have two things needed to be clarified:
|
Yeah, I thought about your point 1 a bit. Seems like there are two options: For the other question, I think |
I think the first form is better since some clients, e.g. ess-dive folks, have scripts based on this form. So it can minimize their changes if we adopt the first form. |
I added a NotAuthorized exception for this mn method. |
see PR #1738 |
Based on our discussion about this ticket, we revised the rest api call: PUT /query/solr/index?[all=true] | [&pid={pid}]
PUT /query/solr/index?all=true //Reindex everything
PUT /query/solr/index?all=true&pid={pid1} //pid1 will be ignored. Reindex everything
PUT /query/solr/index?pid={pid1}&pid={pid2} //Reindex pid1 and pid2
PUT /query/solr/index?all=false&pid={pid1}&pid={pid2} //Reindex pid1 and pid2 Methods:
reindexAll(session)
|
Important to note we are treating |
Even as a noun, it then treats the index as a subcollection of the SOLR query engine, which already tries to pass its contents off as a SOLR query string. Is there a potential conflict there? Also, do we want reindex to be scoped only to SOLR? What if we switch from SOLR to elasticsearch in the future? Reinfecting would still be needed. Can we separate the API from the implementation? I'd prefer that. |
Good points. Would this simpler form work, in that case? PUT /index?[all=true] | [&pid={pid}] |
I think I like that better. It treats the index as a resource (which it is), and makes the service implementation independent. Let's discuss during tomorrow's backend call to finalize. |
Final version:Rest Calls: PUT /index/{pid1}
PUT /index[/]?[all=true] | [&pid={pid}]
PUT /index?all=true //Reindex everything
PUT /index?all=true&pid={pid1} //pid1 will be ignored. Reindex everything
PUT /index/?pid={pid1}&pid={pid2} //Reindex pid1 and pid2
PUT /index/?all=false&pid={pid1}&pid={pid2} //Reindex pid1 and pid2 Two Methods:
reindexAll(session)
|
Did we decide not to support the following example? Or should we allow it as valid? PUT /index/{pid1} |
We didn't mention to support this example.
|
I requested that we support the standard REST syntax. We discussed that on the call today @taojing2002
|
I just remembered we discussed the issue on |
Now, we disabled the
reindex
andreindexall
methods on the olde Metacat API. They are very helpful. I am planning to add them into the Metacat Admin API. They are still only called by the Metacat administrators.The text was updated successfully, but these errors were encountered: