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

[BUG] Async model usage needs to import private _async module #635

Open
pattrickrice opened this issue Dec 18, 2023 · 6 comments
Open

[BUG] Async model usage needs to import private _async module #635

pattrickrice opened this issue Dec 18, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@pattrickrice
Copy link
Contributor

pattrickrice commented Dec 18, 2023

What is the bug?

A clear and concise description of the bug.

If I want to use AsyncDocument for example, it seems like I need to import from a private module (private indicated by _ prefix.

from opensearchpy._async.helpers.document import AsyncDocument

How can one reproduce the bug?

Steps to reproduce the behavior.

from opensearchpy._async.helpers.document import AsyncDocument

What is the expected behavior?

If this is not expected to be private, the suggestion would be to remove the _. Otherwise it appears that use of the async module is discouraged.

from opensearchpy.async.helpers.document import AsyncDocument

What is your host/environment?

Operating system, version.
n/a

Do you have any screenshots?

If applicable, add screenshots to help explain your problem.
n/a

Do you have any additional context?

Add any other context about the problem.

n/a

@pattrickrice pattrickrice added bug Something isn't working untriaged Need triage labels Dec 18, 2023
@saimedhi saimedhi removed the untriaged Need triage label Dec 18, 2023
@dblock
Copy link
Member

dblock commented Dec 20, 2023

Yes, this looks incorrect. Would appreciate a PR. Are there other types in this case? Does anything have to be _async?

@drewbrew
Copy link

drewbrew commented Jan 2, 2024

from opensearchpy.async.helpers.document import AsyncDocument

This is invalid because async is a reserved keyword. Either make it async_ or something else entirely, I think.

@pattrickrice
Copy link
Contributor Author

Oh interesting, I hadn't considered that. Is there a reason the async classes aren't exported in the main barrel export? https://github.com/opensearch-project/opensearch-py/blob/main/opensearchpy/__init__.py

It seems like most of them are prefixed with Async{ClassName}; so, I'd assume there wouldn't be conflicts in adding them there?

@dblock
Copy link
Member

dblock commented Jan 3, 2024

Thanks for pointing this out @drewbrew. I guess I don't know the answer to this, let's work backwards from what we want?

@pattrickrice
Copy link
Contributor Author

Just verified that yes naming a module async will not work:

Traceback (most recent call last):
  File "<frozen runpy>", line 189, in _run_module_as_main
  File "<frozen runpy>", line 112, in _get_module_details
  File "/private/tmp/test/__init__.py", line 1
    import async
           ^^^^^
SyntaxError: invalid syntax

Hmm working backwards... It would be interesting if async stuff were defined next to their sync counter parts rather than being grouped in their own directory? That's sort of a larger reorganization proposal. Other easier alternatives could be _ as a suffix (async_) which wouldn't conventionally mean private in python, expanding async to asynchronous, or using a multi-word like async_opensearch.

@dblock
Copy link
Member

dblock commented Feb 1, 2024

@pattrickrice Is this based on the code duplication? Much of it is generated, or should be, so I'd start by deleting all the API implementations and focusing on a new code generator from spec that spits out any layout that you think makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants