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

[Backport 2.x] Introduced writing layer, getting rid of writing logic that uses an absolute path in the filesystem. #2248

Conversation

0ctopus13prime
Copy link
Contributor

Description

Backporting #2241 to 2.x branch.

…bsolute path in the filesystem.

Signed-off-by: Dooyong Kim <[email protected]>
@0ctopus13prime 0ctopus13prime changed the title Introduced writing layer, getting rid of writing logic that uses an absolute path in the filesystem. [Backport] Introduced writing layer, getting rid of writing logic that uses an absolute path in the filesystem. Nov 5, 2024
@0ctopus13prime
Copy link
Contributor Author

Will check the failure in Windows

@VijayanB VijayanB mentioned this pull request Nov 7, 2024
5 tasks
@0ctopus13prime
Copy link
Contributor Author

The failure does not seem like related to the change.
It failed to get a value from mocking instance.

We set the mocking here, but for some reasons, the mocked value is not being returned in Windows.
I'm suspecting this is a concurrent issue in testing.

Model mockModel = new Model(modelMetadata1, modelBlob, modelId);
when(modelDao.get(modelId)).thenReturn(mockModel); <---------------- This should return 'test-model'
when(modelDao.getMetadata(modelId)).thenReturn(modelMetadata1);

Error :

  1> java.lang.IllegalArgumentException: Model ID 'test-model' is not created.
  1> 	at org.opensearch.knn.indices.ModelUtil.getModelMetadata(ModelUtil.java:54)
  1> 	at org.opensearch.knn.common.FieldInfoExtractor.extractKNNEngine(FieldInfoExtractor.java:42)
  1> 	at org.opensearch.knn.index.codec.util.KNNCodecUtil.getNativeKNNEngine(KNNCodecUtil.java:126)

@0ctopus13prime
Copy link
Contributor Author

Reran the test.
If it is still failing, then will fix the code in Windows and reraise the PR!

@0ctopus13prime
Copy link
Contributor Author

All tests passed 😅
Will merge the PR.
Also will create a new issue regarding the mocking failure.

@navneet1v navneet1v changed the title [Backport] Introduced writing layer, getting rid of writing logic that uses an absolute path in the filesystem. [Backport 2.x] Introduced writing layer, getting rid of writing logic that uses an absolute path in the filesystem. Nov 7, 2024
@navneet1v navneet1v merged commit 2e603a1 into opensearch-project:2.x Nov 7, 2024
104 of 106 checks passed
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.

3 participants