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

Remove legacy Archiver and backup api #605

Merged

Conversation

aprudhomme
Copy link
Contributor

Remove the deprecated legacy Archiver and the legacy backup system in the 1.0.0-SNAPSHOT branch.

The Archiver interface in general is outdated, and doesn't really fit its usage anymore. I have proposed a RemoteBackend interface to replace it, which tries to better abstract away the backend implementation from the other nrtsearch components. This interface is used to replace the legacy archiver for warming query interactions.

The next steps would be to move the incremental archiver interactions (global state, index state, index data) to use this interface as well. This should make it much easier to make future changes related to backend resource structure.

@aprudhomme aprudhomme requested a review from sarthakn7 October 30, 2023 23:58
@aprudhomme aprudhomme marked this pull request as draft October 31, 2023 17:54
@aprudhomme aprudhomme marked this pull request as ready for review November 1, 2023 18:53

@Test
public void testUploadFile() throws IOException {
File uploadFile = S3_PROVIDER.getTemporaryFolder().newFile("upload_file");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems weird to get TemporaryFolder from S3Provider and use it. Unless someone reads the code, they might just put things in the s3 directory via the TemporaryFolder without meaning to. Maybe just create TemporaryFolder here for this class and remove the getter?

@aprudhomme aprudhomme requested a review from sarthakn7 January 2, 2024 23:50
@aprudhomme aprudhomme merged commit 8b0e255 into Yelp:1.0.0-SNAPSHOT Jan 3, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants