I've got to think this one through. It means we either have to add the item labels into the database as well (since the current code isn't doing a "live" pull on each request now) or just a flag on RTC labeled items.

I think it will be enough to just to display the github labels in the com_patchtester

As the CMS maintainers are sometimes excruciatingly slow (weeks or months not day) to commit a PR this is resulting in an average of over 30 RTC issues at any time

A column with latest travis results and link to it would be cool too.

Looking at GitHub's API data, I'm half tempted to close this as "won't implement".

First, remember GitHub has a rate limited API allowing 60 anonymous requests per IP per hour and 5000 authenticated requests per hour. As of this writing, the CMS has 343 open pull requests.

Right now, to pull in the list of pull requests the code loops over the pulls API endpoint with a 100 item batch size until it reaches a page that contains zero data. With the CMS' current open pull requests, this requires 5 API requests (the first 3 are full 100 item lists, the 4th is the remaining 43 items, and the 5th is the zero data request).

According to https://developer.github.com/v3/pulls/#list-pull-requests pull requests are not sent with their label or status data so this requires additional API lookups to get the additional data. Using the CMS' current open pull requests, this would require the 5 API requests to get the full list of items then for each item it would require two additional lookups (one query to get the labels on the item and one to get the test status). This means it would take 691 API requests to get all of this data (or 348 for only one of the two suggested items), and would also most likely require smaller batch sizes to ensure there aren't any HTTP timeouts during the retrieval process, so let's just call it 700 at max for the sake of discussion.

This immediately means someone cannot use the patchtester without a GitHub account; at least in the current state you can pull down the pull requests without needing authentication.

Also note that the issue tracker does not have an API so pulling the data from there instead of direct from GitHub isn't an option. It also limits the patch tester to only issue tracker supported repos (a rarely used feature is its ability to pull data from any GitHub repo, there's just a requirement that the repo match the CMS' folder structure to apply the patch; you can actually test patches for the patch tester using the patch tester because of this).

@mbabker i understand what you mean.

It is probably more work, but is somehow possible two have a table layout for users with github account (with extra fields) and other (the one that exists now) for users without github account?
Of course the fetching process would be different too.

Probably is not an option because of the 700 API calls you refered, but at least user would continue to test without Github Account.

I'm half tempted to close this as "won't implement".
Reading again your comments ... maybe 80% tempted ...

Personally, I'd rather not get into that type of conditional. I'd rather impose a credentials requirement versus an excessively dynamic fetch process (which then affects the list view and how the data is retrieved) based on whether credentials are available.

If it wasn't for GitHub's rate limited API, I would love to see the additional info, c'est la vie !

Me too.

Ps you can go to options and use your own github credentials
On 20 Jan 2016 3:22 am, "photodude" notifications@github.com wrote:

If it wasn't for GitHub's rate limited API, I would love to see the
additional info, c'est la vie !

—
Reply to this email directly or view it on GitHub
#88 (comment)
.

I'm still thinking this over. I don't want to break being able to use this anonymously and I don't want to introduce overly complex conditionals based on anonymous versus authenticated data pulling, but it'd be nice to add some extra data that the labels give.

Maybe I'm just thinking crazy, but To avoid introducing "overly complex conditionals" in the current view; Could two views be available? One for anonymous and another for authenticated???

On the one hand, haveing two seperate view should give us a bit more flexibility to pull in the additional API items without complex conditionals in a single view. but it would add some complexity to maintenance since there will be some level of duplication that would need to be maintained.

The views aren't the issue. Conditionally you're really just adding one column to the layout (RTC yes/no). The problems start to crop up in the fetching mechanism.

Instead of just looping over 4 or 5 requests to the get pull requests endpoint, if you're authenticated, you're making one request to that endpoint (for a smaller data set), then on each item that comes back you have to ping the endpoint to get all labels for an issue and add whatever data gets used out of that into the object that get inserted into the database. And it has to avoid timeout issues. So instead of 4 or 5 requests for 100 items each, now each request for the pull requests endpoint would need to max somewhere around 20 items. So in the scope of one request, you end up with 21 HTTP pings to the GitHub API (one for the group of items and one each for each item's labels). The other option would be to do the 5 requests to get all the issues then get the labels and update records, but that's inefficient too because now you're running a batch of insert queries followed by a batch of update queries.

To me it looks more error prone to connection timeouts and errors and is more liable to get GitHub's attention with ~600 API pings in a very short period.

Maybe I'm reading the API wrong, but it looks like the List Pull Requests has some useful things in the response (note: I'm looking at the example in the API and not a real response)
to me it looks like milestone, git_tags, labels, tags are all avaialble in the response. If that's right there is one call and parsing the response (But I've only spent about 15minutes looking at the API's example response and I have not looked at a real response from our actual data; I totally could be misreading what it's saying in the example response).

This gist shows the data for the latest PR on the CMS - https://gist.github.com/mbabker/cc97d3aea98dbcbe5007

It has links to a bunch of the other relevant data, but labels are not included on pull requests. They ARE part of the payload for issues though.

Updated the gist a bit. pull.json is a single PR pulled from the API, pull-list.json is the same pull request via the list pull requests endpoint (filtered to only return one PR).

Well that's an interesting thought, Should we use the "issues" list and filter or truncate the data via the pull_request key? It's more data to be pulled and processed, but if it includes the relivant data we want in the payload; the cruft from issues that are not PR's in the response might be worth dealing with.

I'll take a look at the Gist to see the existing response data.

As long as GitHub doesn't deprecate returning pull requests as part of the list of issues in a repository, it's conceivably possible - https://developer.github.com/v3/issues/#list-issues-for-a-repository

I gave a look into using the issues API. We lose #103 and #121 if we go that route unless we want to make more API requests just for the commit SHA.

FWIW master...issues-api is the needed changes to go that route.

RTC state based on whether the label is assigned is now stored and that can be filtered. Consider this done/closed.