redhat-aqe/review-rot

review-rot's Issues

Authenticating on gitlab.cee produces 404 errors

review-rot encounters 404 errors when authenticating on gitlab.cee. Testing indicates this does not occur if GitLab API is forced to v3 instead of v4.

Fix fetching requests for given user/project name

Review-rot was supposed to fetch pull requests for a given repository or for all repositories for a given user/project name. It looks like, this feature has been over written in one of the previous merges.
E.g,

• type: github
  token: my_token
  host: null
  repos:
  • nirzari

For above configuration, it should ideally print pull requests for all repositories for user 'nirzari'.
Instead, I am getting an Exception: Repository nirzari not found for user nirzari.
Likewise for other git services.

Add support for gerrit!

I would be great to be able to list open gerrit reviews too

Display +1s

It would be great to be able to see +1s and -1s

Add doc strings to missing places

Doc strings are missing at some places from recent contribution/modification. e.g, gerritstack.py, pagurestack.py etc.

Possibility to ignore WIP PRs?

It's common convention to add "WIP" suffix to PRs which are work in progress. These PRs may go stale faster than regular PRs. It would be great if review-rot had an option to ignore such PRs.

For email notifications, for example, it may desirable to completely ignore them.

Build container images?

Hi!

There's ralphbean/f2-reviewrot-openshift which has all the configuration needed for deploying review-rot in OpenShift. Although much of the configuration is specific to the Factory team, the container images are not.

There's one for frontend
And another for backend.

Frontend is just an NGINX app that serves the static files for review-rot.
Backend is a CronJob that runs periodically to generate the data.json.

We currently build these container images locally in our OpenShift project which is fine and all, but it doesn't really allow us to share it with others.

Would you be interested in having these images being built and shared in something like quay.io?

I don't mind doing the work, but I want to make sure you'd be interested before going ahead with it.

Fix test BaseServiceTest.test_has_new_comments

The test had been failing last 4 commits.

Make script PEP8 compatible

I own an internal ticket to apply pep8 to review-rot.
(apparently my team is the new maintainer of review-rot)

The activity happens in PR #122

Internally at RH the activity is tracked in:

• PVDRASP-199
• PVDRASP-203

Please review.
Thanks in advance!

Support phabricator

I'm not sure exactly how this should work, but I'd like to be able to pull open reviews from a phabricator instance.

We have one internally. I'll supply the link on an internal ticket.

Get this working with that "other" gitlab instance.

This works with gitlab.cee but it doesn't work with gitlab.infra. ;)

Anyone know why?

RFE: Custom sorting formula

I can imagine that different teams have different preferences on which PRs need to be taken care of first. It would be difficult to support various sorting criteria, but I can imagine that having some crucial numbers (PR age, number of comments, days since last comment, number of code changes (patch size), project importance (by project tags/topics) etc. - all converted to some variable names), one could easily set his/her own formula for calculating sorting priority. The formula would be stored in the config file, calculated for each PR. The list of PRs would be sorted accto the calculated values.

Improve tox configuration

The old tox.ini configuration is sufficient. It should be updated and include style test and unittest with minimal coverage of 60%.

Do not send empty emails

If there are no reviews identified, review-rot sends out an empty email. This is confusing. Can we have review-rot not send an email in this case?

Alternatively, review-rot could send a "congratulations" email for not having anything to review 😃

Gracefully handle remote API failures

reviewrot fails completely if for any reason one of the remote endpoint fails. It may still be worthwhile to get partial data than no data.

Implement graceful handling of remote API failures, with a note that not all data could be obtained.

Default configurations

Would be great to enhance ~/.reviewrot.yaml to also allow setting defaults for CLI arguments, such as format. That way users can simply use review-rot without any arguments for common usage.

If an argument is defined in config, and also passed in from CLI, it should honor CLI value.

Redirect the output to a file waits indefinitely

While taking user input for config re-write, program waits until input is given. If we redirect the output to a file, it waits indefinitely as there is no option to provide input in case of old config file.
Because of this, cron job used to produce web page will get stuck if user is using old config file.

Implement retry-backoff for github API

Github API has a rate limit in place and reviewrot breaks that, leading to the following exception:

DEBUG:github.Requester:GET https://api.github.com/users/nirzari  {'User-Agent': 'PyGithub/Python'} None ==> 403 {'status': '403 Forbidden', 'x-ratelimit-remaining': '0', 'x-github-media-type': 'github.v3; format=json', 'content-security-policy': "default-src 'none'", 'x-content-type-options': 'nosniff', 'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type', 'transfer-encoding': 'chunked', 'x-github-request-id': 'REDACTED', 'strict-transport-security': 'max-age=31536000; includeSubdomains; preload', 'server': 'GitHub.com', 'content-encoding': 'gzip', 'x-ratelimit-limit': '60', 'x-xss-protection': '1; mode=block', 'date': 'Tue, 02 Jul 2019 08:14:42 GMT', 'access-control-allow-origin': '*', 'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin', 'content-type': 'application/json; charset=utf-8', 'x-frame-options': 'deny', 'x-ratelimit-reset': '1562055316'} {"message":"API rate limit exceeded for 66.187.233.202. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","documentation_url":"https://developer.github.com/v3/#rate-limiting "}
Traceback (most recent call last):
  File "/usr/bin/review-rot", line 4, in <module>
    __import__('pkg_resources').run_script('review-rot==1.0', 'review-rot')
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 666, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1460, in run_script
    exec(script_code, namespace, namespace)
  File "/usr/lib/python2.7/site-packages/review_rot-1.0-py2.7.egg/EGG-INFO/scripts/review-rot", line 286, in <module>
    
  File "/usr/lib/python2.7/site-packages/review_rot-1.0-py2.7.egg/EGG-INFO/scripts/review-rot", line 105, in main
    
  File "build/bdist.linux-x86_64/egg/reviewrot/githubstack.py", line 46, in request_reviews
  File "build/bdist.linux-x86_64/egg/github/MainClass.py", line 233, in get_user
  File "build/bdist.linux-x86_64/egg/github/Requester.py", line 275, in requestJsonAndCheck
  File "build/bdist.linux-x86_64/egg/github/Requester.py", line 286, in __check
github.GithubException.RateLimitExceededException: 403 {u'documentation_url': u'https://developer.github.com/v3/#rate-limiting' , u'message': u"API rate limit exceeded for 66.187.233.202. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)"}

There are several things to do here, in order of importance:

• implement retry with exponential backoff for github API
  • this should work in the general case
• parse the rate limit response from Github and respect that (wait for the designated period) and re-try the request
• add support for authenticated requests to Github API -- this permits a higher rate of polling Github API

More details in the specified link:

• https://developer.github.com/v3/#rate-limiting

Pagure failure

One of the tests in test.py -> test_request_review_incorrect_project_with_repo for pagure fails.
Also I can not get pagure section of review-rot working correctly. Looks like something got broken in one of the previous merges.

No. of comments in review-rot's github response are incorrect

The no. of comments in review-rot's github response are incorrect
For pull request#40, currently there are 23 comments. It is showing 1.

Show date of latest comment and enable sorting by it

Hello, can we show date of latest comment in Open PR overview? I would like to sort PRs by activity (last comment) instead of when PR was opened.

Display GitLab ThumbsUp

It would be great it you could display GitLab's ThumbsUp.

Invalid json format

With this configuration:

git_services:
  - type: pagure
    host: pagure.io
    repos:
    - fm-orchestrator

review-rot output invalid json:

🐚 review-rot -c configuration.yaml -f json
[
None
{
  "user": "merlinm",
  "title": "WIP module scratch builds",
  "url": "https://pagure.io/fm-orchestrator/pull-request/1136",
  "relative_time": "5 days",
  "time": 1549664519.0,
  "comments": 4,
  "type": "PagureReview",
  "image": "https://seccdn.libravatar.org/avatar/703db885ef67e2f87122e6ddcdf16a0b4352b857a4f99dd0896ce8c78acf3382?s=64&d=retro",
  "last_comment": {
    "author": "mvadkert",
    "created_at": 1550075633.0
  }
},
{
  "user": "jkaluza",
  "title": "Fix the handling of modulemd files with unicode characters.",
  "url": "https://pagure.io/fm-orchestrator/pull-request/1144",
  "relative_time": "1 day 3 hours",
  "time": 1550001036.0,
  "comments": 17,
  "type": "PagureReview",
  "image": "https://seccdn.libravatar.org/avatar/1ecc757d81ddcab5c53f2d5741595dece34d1e6519a39d57a8fa4af6b8b12b11?s=64&d=retro",
  "last_comment": {
    "author": "jkaluza",
    "created_at": 1550080938.0
  }
}
]

Notice the odd None on second line.

Replace -s -d -n with --until & --since

Could you replace CLI arguments -s, -d and -n with something more simple and useful, like since and until.

Something similar to this:

--since {ISO 8601}
--until {ISO 8601}

You don't have to use standard ISO 8601.

IRC header confuses karma bots

When review-rot is configured to send IRC notifications, it starts the report like this:

--------------------------------------------- Code Review Reminder ---------------------------------------------

The dashes cause karma bots to reply with:

------------------------------------------- now has -26 points of karma.

Can we change - to perhaps #? Or better yet, maybe just don't add a header/footer at all?