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

/list endpoint is not listing morph modes #206

Open
jonorthwash opened this issue Dec 21, 2022 · 8 comments · May be fixed by #230
Open

/list endpoint is not listing morph modes #206

jonorthwash opened this issue Dec 21, 2022 · 8 comments · May be fixed by #230

Comments

@jonorthwash
Copy link
Member

I have a docker image with a single apertium language module.

When /list is queries explicitly for analysers and generators, it reports their availability:

$ curl 'http://localhost:2738/list?q=analyzers'
{"xyz": "xyz-morph"}
$ curl 'http://localhost:2738/list?q=generators'
{"xyz": "xyz-gener"}

Use of the modes also works. However, when I query /list without specifying type, it does not report the analysers and generators:

$ curl 'http://localhost:2738/list'
{"responseData": [], "responseDetails": null, "responseStatus": 200}

According to the documentation, /list should be returning all mode names, not just language pairs.

I can't figure out if this unexpected (for me) behaviour is a bug, incorrect/out-of-date documentation, or a mis-configuration on my part.

@Mohab96
Copy link

Mohab96 commented Mar 16, 2024

According to the documentation, you're right, but according to this piece of code, I think it should return only the pairs.
image
It can be found in "apertium_apy\handlers\list_modes.py"

And the live version of apertium-apy supports my claims, so if you tried to call this https://apertium.org/apy/listPairs and https://apertium.org/apy/list they will give you the exact same result.

@jonorthwash
Copy link
Member Author

Try beta.apertium.org. I believe other modes are listed. They're disabled in the config on the production site.

@jonorthwash
Copy link
Member Author

according to this piece of code, I think it should return only the pairs.

I think you've found the source of the issue. This could be repaired easily then. The question is then whether the docs or the code is right. @sushain97, @TinoDidriksen, @unhammer?

@gopalM-3
Copy link

gopalM-3 commented Mar 16, 2024

Both the /list and /listPairs routes are handled by the same route-handler ListHandler, that's where the problem lies.

Not necessarily. A well-written function can be used for related but different things.

As of what I've checked, even https://apertium.org/apy/listPairs?q=analyzers returns the JSON having the analyzers, which isn't expected behaviour.

That is exactly expected behaviour. Everything looks good here.

gopalM-3 added a commit to gopalM-3/apertium-apy that referenced this issue Mar 16, 2024
@gopalM-3 gopalM-3 linked a pull request Mar 16, 2024 that will close this issue
@gopalM-3
Copy link

@jonorthwash please check if PR #230 does what you're looking for, let me know if any further adjustments are to be made.

@Mohab96
Copy link

Mohab96 commented Mar 16, 2024

@jonorthwash please check if PR #230 does what you're looking for, let me know if any further adjustments are to be made.

According to the docs, this is not what this endpoint is intended to do, nothing in the documentation mentioned that if no query parameter is passed then it gets all the available modes. It is mentioned in the function of the endpoint /list, however, it is not shown in the output section so you don't know whether this is right or not in business terms.

@gopalM-3
Copy link

gopalM-3 commented Mar 16, 2024

@Mohab96 The issue's author highlighted that, based on the documentation, the expected behavior of the /list endpoint is to return all mode names, not solely language pairs.

According to the documentation, /list should be returning all mode names, not just language pairs.

Consequently, I pursued this approach. Let's await further clarification from the author to ensure alignment with expectations.

@sushain97
Copy link
Member

According to the documentation, /list should be returning all mode names, not just language pairs.

I think the original intent was that /list, /list?q=pairs and /listPairs all returned identical output.

The documentation does not make the behavior of /list clear.

We could take one of three options:

  1. Clarify the documentation to match the current implementation.
  2. Change the semantics of /list similar to fixed #206 #230.
  3. Maintain the semantics of /list but add a new /list?q=all similar to fixed #206 #230.

I'm biased against (2) since it's backwards incompatible (albeit likely not a big deal). Option (3) would further complicate the potential return types of /list which isn't ideal, IMO. It's already weird enough. That would leave option (1) as by default. Users like the APy build currently just issue 4 requests if they need all the modes. But, if that's particularly undesirable, (3) isn't terrible.

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 a pull request may close this issue.

4 participants