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

[Infrastructure] BWC tests for Chunking Processor #647

Closed
vibrantvarun opened this issue Mar 18, 2024 · 9 comments
Closed

[Infrastructure] BWC tests for Chunking Processor #647

vibrantvarun opened this issue Mar 18, 2024 · 9 comments
Assignees

Comments

@vibrantvarun
Copy link
Member

BWC tests for chunking processor wrt 607

@vibrantvarun
Copy link
Member Author

@yuye-aws Can you add the bwc tests for the feature.

@yuye-aws
Copy link
Member

This feature implements a new processor for text chunking, which does not break any existing functionalities. I do not think there is any potential risk of breaking the backward compatibility. You can also verify that with my code. Can we simply close this issue?

@yuye-aws
Copy link
Member

Feel free to raise any concerns regarding the necessity of BWC test.

@vibrantvarun
Copy link
Member Author

@yuye-aws We need BWC tests for future releases. Like when 2.14 will be released then we need to run bwc test for the same functionality.

@vamshin vamshin removed the untriaged label Mar 21, 2024
@vibrantvarun vibrantvarun changed the title BWC tests for Chunking Processor [Infrastructure] BWC tests for Chunking Processor Mar 21, 2024
@yuye-aws
Copy link
Member

Hi @vibrantvarun , can I follow your PR in this issue? Is BWC test a hard requirement for 2.14 release?

@navneet1v
Copy link
Collaborator

Hi @vibrantvarun , can I follow your PR in this issue? Is BWC test a hard requirement for 2.14 release?

I would say right now as a best practice we should add the BWC test. This will help in future to catch bugs if we are making changes in these processors or if any breaking change comes from Opensearch core.

I am coming up with the more detailed developer guide for adding new features in neural search plugin and I will include this as a point there. We can discuss whether BWC should be a hard requirement there and lets other maintainers share their thoughts.

@yuye-aws
Copy link
Member

yuye-aws commented Apr 1, 2024

Sure. I will implement the BWC tests.

@yuye-aws
Copy link
Member

yuye-aws commented Apr 2, 2024

Hi @navneet1v and @vibrantvarun ! This PR implements the BWC test for text chunking processor. Can you help review?

@vibrantvarun
Copy link
Member Author

Reviewed and added the comments.

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

4 participants