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

[PROPOSAL] Merge opensearch-dsl-py into this repository #194

Closed
harshavamsi opened this issue Sep 7, 2022 · 15 comments · Fixed by #287, #293 or #329
Closed

[PROPOSAL] Merge opensearch-dsl-py into this repository #194

harshavamsi opened this issue Sep 7, 2022 · 15 comments · Fixed by #287, #293 or #329

Comments

@harshavamsi
Copy link
Collaborator

What are you proposing?

Merge the opensearch-dsl-py repo here as an additional package. Users would be able to import both the dsl libraries and the python-client since dsl functions always require importing the python client.

How did you come up with this proposal?

Currently the ruby client has multiple gems that are published independently of each other while living in the same repository.

What is the user experience going to be?

The user experience would be unchanged from how it is today, however it will make developer experience much better. Maintainers will have to release one less client as both the dsl libraries and the python client can be released together.

Why should it be built? Any reason not to?

Can't think of any.

What will it take to execute?

Porting over the client here should be fairly straightforward. It is unclear at this time what it takes to merge the source here and how new releases will happen.

What are remaining open questions?

  • Will this make discovery of the dsl client harder?
  • Is this even feasible? Happy to be proven wrong if not.
@wbeckler
Copy link
Contributor

@saimedhi can you take a look?

@saimedhi
Copy link
Collaborator

I will work on it.

@saimedhi
Copy link
Collaborator

These are steps I am thinking to follow. Correct me if these are not feasible.

  1. "opensearch_dsl_py" and "opensearch_dsl_py_tests" will be added as new folders in opensearch-py repo.
  2. Changelog will be same for both.
  3. The following files will get merged- user guide, security.md, releasing.md, OpenSearch.svg, README.md, NOTICE.txt, MANIFEST.in, MAINTAINERS.md, LICENSE.txt, CONTRIBUTING.md, compatibility.md, CODE_OF_CONDUCT.md, AUTHORS, ADMINS.md, .whitesource, ,github.
  4. Can these files be merged too- setup.py, setup.cfg, noxfile, utils, .ci/opensearch, .gitignore, .ci/opensearch?
  5. Github workflow should be setup in a way to skip opensearch_dsl_py_tests if opensearch_dsl_py is not used.
  • Alternate possibility is to create a folder in "opensearch-py" repo which has all the "opensearch_dsl_py" content in it.

@wbeckler
Copy link
Contributor

I think Step 5 can be deleted, as running the tests will always matter for the build.

@saimedhi
Copy link
Collaborator

@wbeckler can you please give your thoughts about the alternate method mentioned above. As in opensearch-ruby, can we setup completely different folder for opensearch-dsl-py in opensearch-py repo which has its own readme, changelog, etc.

@saimedhi
Copy link
Collaborator

@opensearch-project/clients can everyone please give your inputs.

@saimedhi
Copy link
Collaborator

Final approach that I am thinking to follow is:

  • Merging opensearch-dsl-py features completely into opensearch-py without having separate repo or folder.
  • Coming to how it is going to work

Before:
from opensearchpy import OpenSearch
from opensearch_dsl import Search
After:
from opensearchpy import OpenSearch, Search


@dblock
Copy link
Member

dblock commented Jan 26, 2023

This is a good plan. I agree that opensearch-dsl-py should simply get absorbed into opensearch-py and disappear as a thing.

@saimedhi
Copy link
Collaborator

saimedhi commented Feb 7, 2023

kindly let me know If any of the following steps is incorrect or not as expected.

  • Merge opensearch-dsl-py to opensearch-py (including tests).
  • Create async version of opensearch-dsl.
  • Create an UPGRADING.md file as requested here.
  • Update documention in the github opensearch-py repo.

@dblock
Copy link
Member

dblock commented Feb 7, 2023

We'll need a version increment, major or not?

@saimedhi
Copy link
Collaborator

saimedhi commented Feb 7, 2023

We'll need a version increment, major or not?

I am not sure. @wbeckler can you please tell what do you think.

@dtaivpp
Copy link

dtaivpp commented Feb 7, 2023

@saimedhi we aren't changing the import structure for the current opensearch-py library right? Just adding the DSL so it can be imported something like import opensearchpy.dsl right? So long as that's the case I feel like we should be fine with a minor version bump.

@dblock
Copy link
Member

dblock commented Feb 7, 2023

If there's a backwards incompatible change it's a major version (e.g. 3.0), otherwise it's a minor (e.g. 2.3) version bump.

@saimedhi
Copy link
Collaborator

saimedhi commented Feb 7, 2023

@saimedhi we aren't changing the import structure for the current opensearch-py library right? Just adding the DSL so it can be imported something like import opensearchpy.dsl right? So long as that's the case I feel like we should be fine with a minor version bump.

@dtaivpp yes, as of now the idea is to have seperate folder for dsl in opensearchpy

@dtaivpp
Copy link

dtaivpp commented Feb 8, 2023

@dblock I think we are saying the same thing. Seems this will all be adding a new module that shouldn’t break backwards compatibility so probably a minor version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment