Added implementations for DataStore removal and copy methods.#1478
Merged
Added implementations for DataStore removal and copy methods.#1478
Conversation
rfecher
reviewed
Jan 2, 2019
| final short internalAdapterId, | ||
| String indexName ) { | ||
|
|
||
| adapterCache.remove(internalAdapterId); |
Contributor
There was a problem hiding this comment.
this line should be removed
rfecher
reviewed
Jan 2, 2019
| baseOperations.deleteAll( | ||
| index.getName(), | ||
| adapter.getTypeName(), | ||
| "", |
Contributor
There was a problem hiding this comment.
seems like this line should stay the way it was
rfecher
reviewed
Jan 2, 2019
| writer.write(it.next()); | ||
| } | ||
| writer.close(); | ||
| it.close(); |
Contributor
There was a problem hiding this comment.
try with resources is used, no need to close
rfecher
reviewed
Jan 2, 2019
| indices); | ||
|
|
||
| // copy the data over for the given adapter type / indices | ||
| for (int k = 0; k < indices.length; k++) { |
Contributor
There was a problem hiding this comment.
this loop is unnecessary and actually seems to write the same data multiple times (querying by index is not necessary)
rfecher
reviewed
Jan 2, 2019
| final String indexName = query.getIndexQueryOptions().getIndexName(); | ||
| final boolean isAllIndices = query.getIndexQueryOptions().isAllIndicies(); | ||
|
|
||
| // if typeNames are not specified, then it means 'everything' as well |
Contributor
There was a problem hiding this comment.
this isn't true if there are query contraints in the query.
rfecher
reviewed
Jan 2, 2019
| typeName).indexName( | ||
| indices[k].getName()); | ||
| try (CloseableIterator<?> it = query(qb.build())) { | ||
| Writer writer = other.createWriter(typeName); |
Contributor
There was a problem hiding this comment.
try with resources for the writer
rfecher
reviewed
Jan 2, 2019
| // TODO Auto-generated method stub | ||
|
|
||
| // remove the given index for all types | ||
| CloseableIterator<InternalDataAdapter<?>> it = adapterStore.getAdapters(); |
rfecher
reviewed
Jan 2, 2019
| final short adapterId = markedAdapters.get(i); | ||
| baseOperations.deleteAll( | ||
| indexName, | ||
| "", |
rfecher
reviewed
Jan 2, 2019
| // Remove all the data for the adapter and index | ||
| baseOperations.deleteAll( | ||
| indexName, | ||
| "", |
rfecher
reviewed
Jan 2, 2019
| for (int i = 0; i < indexNames.length; i++) { | ||
| baseOperations.deleteAll( | ||
| indexNames[i], | ||
| "", |
rfecher
reviewed
Jan 2, 2019
| @Override | ||
| public void deleteAll() { | ||
| deleteEverything(); | ||
| // Clear all the data and stats, but leave adapters/indices alone |
Contributor
There was a problem hiding this comment.
is that the desired behavior? it is nice to have a "wipe everything" op as well then
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added necessary implementations for removal of indices and types and also copyTo methods. Work should perform all required house keeping duties as well. This PR addresses issue #1440.