psss / did

What did you do last week, month, year?
https://did.readthedocs.io/
GNU General Public License v2.0
246 stars 104 forks source link

Add the new `phabricator` plugin #279

Closed kwk closed 1 year ago

kwk commented 1 year ago

This allows you to connect to a phabricator or phorge instance and get stats from it for multiple usernames.

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging ac56dd27ca0e479fc8ee7bd893d52f62a13b8fae into f3a408130f2e255f0e64f852d101da79fe32a62b - view on LGTM.com

new alerts:

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging 01b5205260653584e917cf44a896eb969b0c851e into f3a408130f2e255f0e64f852d101da79fe32a62b - view on LGTM.com

new alerts:

kwk commented 1 year ago

@psss I'd like to debug what went wrong with the copr builds but I guess the logs have timed out. Can you retrigger a build?

psss commented 1 year ago

/packit test

kwk commented 1 year ago

/packit test

@psss thank you for this. But unfortunately the test run https://github.com/psss/did/pull/279/checks?check_run_id=9476182566 links to the much older copr dashboard: https://dashboard.packit.dev/results/copr-builds/506073 . I wonder how to get to the new results.

psss commented 1 year ago

Seems that packit did not want to rebuild the packages:

The latest build was not successful, not running tests for it.

So I rebased on the latest main which should convince packit to do a fresh rebuild.

psss commented 1 year ago

Fresh logs seem to be there:

>               raise did.base.ConfigError(
                    "Invalid plugin type '{0}' in section '{1}'.".format(
                        type_, section))
E               did.base.ConfigError: Invalid plugin type 'phabricator' in section 'phabricator'.
lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert when merging a371729c1e978e2317da9322b924729787c11a1c into f000e80ec68ea2668c8c3193301805dc469c5e08 - view on LGTM.com

new alerts:

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

kwk commented 1 year ago

/packit test

kwk commented 1 year ago

/packit test

kwk commented 1 year ago

/packit test

kwk commented 1 year ago

Once #284 is merged, rebase this patch. The man page errors should be gone then.

kwk commented 1 year ago

@psss this PR is also ready to be merged if you want to give it a review.

kwk commented 1 year ago

/packit test

kwk commented 1 year ago

@psss I've addressed all pre-commit issues and remove the el8 stuff. Unfortunately the history with this PR is so long that I can hardly pull up the copr build errors. Can we merge this then?

psss commented 1 year ago

I tried to check the new plugin: Created an account, generated a token and added it to the config. I get the folowwing:

Traceback (most recent call last):
  File "/home/psss/.virtualenvs/did/lib/python3.10/site-packages/requests/models.py", line 910, in json
    return complexjson.loads(self.text, **kwargs)
  File "/usr/lib64/python3.10/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/lib64/python3.10/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib64/python3.10/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/psss/.virtualenvs/did/bin/did", line 7, in <module>
    exec(compile(f.read(), __file__, 'exec'))
  File "/home/psss/git/did/bin/did", line 42, in <module>
    did.cli.main()
  File "/home/psss/git/did/did/cli.py", line 223, in main
    user_stats.check()
  File "/home/psss/git/did/did/stats.py", line 157, in check
    stat.check()
  File "/home/psss/git/did/did/stats.py", line 157, in check
    stat.check()
  File "/home/psss/git/did/did/stats.py", line 77, in check
    self.fetch()
  File "/home/psss/git/did/did/plugins/phabricator.py", line 209, in fetch
    for idx, login in enumerate(self.parent.phabricator.login_phids):
  File "/home/psss/git/did/did/plugins/phabricator.py", line 64, in login_phids
    self._login_phids = self._resolve_logins_to_phids(self.logins)
  File "/home/psss/git/did/did/plugins/phabricator.py", line 95, in _resolve_logins_to_phids
    return [user["phid"] for user in response.json()["result"]["data"]]
  File "/home/psss/.virtualenvs/did/lib/python3.10/site-packages/requests/models.py", line 917, in json
    raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
requests.exceptions.JSONDecodeError: [Errno Expecting value] <!DOCTYPE html><html><head>...

And the rest is an html page with message about invalid authentication. Any idea what might be wrong in my setup? Also, could you please handle such error with a more friendly user messsage?

kwk commented 1 year ago

And the rest is an html page with message about invalid authentication. Any idea what might be wrong in my setup? Also, could you please handle such error with a more friendly user messsage?

Yes, and thank you for trying out the plugin yourself. I'll look into this now.

kwk commented 1 year ago

@psss can you retry with d3824a6?

kwk commented 1 year ago

@psss I've made the changes that you requested:

The included tests were also adjusted and to cover the --verbose and non-verbose situations.

psss commented 1 year ago

Great! Thanks much. Squashed and ready for merging.