ryancheley / the-well-maintained-test

Programatically tries to answer the 12 questions from Adam Johnson's blog post https://adamj.eu/tech/2021/11/04/the-well-maintained-test/
Apache License 2.0
23 stars 3 forks source link

json.decoder.JSONDecodeError for https://github.com/scikit-hep/awkward-1.0 #25

Closed matthewfeickert closed 2 years ago

matthewfeickert commented 2 years ago

Describe the bug

the-well-maintained-test v0.6.0 fails with a json.decoder.JSONDecodeError for a url target of https://github.com/scikit-hep/awkward-1.0

$ the-well-maintained-test url 'https://github.com/scikit-hep/awkward-1.0'
1. Is it described as “production ready”?
Traceback (most recent call last):
  File "/venv/bin/the-well-maintained-test", line 8, in <module>
    sys.exit(cli())
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/venv/lib/python3.9/site-packages/the_well_maintained_test/cli.py", line 131, in url
    console.print(Padding(production_ready_check(pypi_url), answer_padding_style, style=answer_style))
  File "/venv/lib/python3.9/site-packages/the_well_maintained_test/utils.py", line 19, in production_ready_check
    response = requests.get(pypi_api_url).json()
  File "/venv/lib/python3.9/site-packages/requests/models.py", line 910, in json
    return complexjson.loads(self.text, **kwargs)
  File "/usr/local/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.9/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 7 column 1 (char 6)

To Reproduce

$ docker run --rm -ti python:3.9 /bin/bash
root@408e9935d421:/# python -m venv venv && . venv/bin/activate
(venv) root@408e9935d421:/# python -m pip --quiet install --upgrade pip setuptools
(venv) root@408e9935d421:/# python -m pip --quiet install the-well-maintained-test
(venv) root@408e9935d421:/# pip freeze | grep the-well-maintained-test
the-well-maintained-test==0.6.0
(venv) root@408e9935d421:/# the-well-maintained-test auth
Create a GitHub personal user token and paste it here:
Personal token: 
(venv) root@408e9935d421:/# the-well-maintained-test url 'https://github.com/scikit-hep/pyhf'  # works just fine
(venv) root@408e9935d421:/# the-well-maintained-test url 'https://github.com/scikit-hep/awkward-1.0'
1. Is it described as “production ready”?
Traceback (most recent call last):
  File "/venv/bin/the-well-maintained-test", line 8, in <module>
    sys.exit(cli())
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/venv/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/venv/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/venv/lib/python3.9/site-packages/the_well_maintained_test/cli.py", line 131, in url
    console.print(Padding(production_ready_check(pypi_url), answer_padding_style, style=answer_style))
  File "/venv/lib/python3.9/site-packages/the_well_maintained_test/utils.py", line 19, in production_ready_check
    response = requests.get(pypi_api_url).json()
  File "/venv/lib/python3.9/site-packages/requests/models.py", line 910, in json
    return complexjson.loads(self.text, **kwargs)
  File "/usr/local/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.9/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 7 column 1 (char 6)

Expected behavior

For an argument of https://github.com/scikit-hep/awkward-1.0 to give valid output.

ryancheley commented 2 years ago

The issue is related to this bit of code:

package = parse_object.path.split("/")[-1]
...
pypi_url = f"https://pypi.org/pypi/{package}/json"

The url from GitHub returns package as awkward-1.0 but on pypi the package is registered as awkward

This is not a case I had considered where the repo name in GitHub does not match the package name in pypi

ryancheley commented 2 years ago

The current design of the package is GitHub first .... but maybe too naive. When the requirements feature was added in v0.6.0 the package is looked up on PyPi and then gets the GitHub url to run the CLI.

Maybe a better way to do it is to start at the PyPi API url first.

First thing might be to output an error / warning message when using the PyPi API url, but that would impact 5 of the 12 questions and make the tool less useful.

ryancheley commented 2 years ago

A good nights sleep may help clarify which direction to go in

ryancheley commented 2 years ago

After a good night’s sleep, and good morning jog, I think that instead of using url as the starting point, we should start with the package name. Then, that can be fed into he PyPi API and that can get the various GitHub API URLs.

So, instead of:

the-well-maintained-test url 'https://github.com/ryancheley/the-well-maintained-test'

We would instead have:

the-well-maintained-test package 'the-well-maintained-test'

We can keep the url argument for backwards compatibility BUT we’d also switch it from being click.argument to a click.option

I’m in the middle of an update for v0.7.0 so I think that I’ll add this change to the roadmap for v0.8.0.

@matthewfeickert what do you think of the proposed change above?

ryancheley commented 2 years ago

The url option has been deprecated and replaced with package. Marking this as closed since the underlying issue has been resolved. See c963b for full change