actually return headers when http method is HEAD

[migueldiascosta]

the message body when using the http HEAD method is empty, this change allows the head() method to actually return the headers (easybuilders/easybuild-framework#2553 (comment))

[stdweird]

@migueldiascosta can you bump the version in setup.py ?
@boegel i guess this needs a backport to EB too?

[migueldiascosta]

@stdweird done. I can submit the same change to the 4.x branch in the EB framework

[stdweird]

@migueldiascosta thanks
wrt the failing tests, i can fix the headers errors (or you can too, see "fix license headers" in https://github.com/hpcugent/vsc-install/#fix-failing-tests)
but can you have a look at the other failing test (as it looks related to the code you modified)?

[migueldiascosta]

@stdweird the test_request_methods error does seem related to my change, since that starts with a call to head() (but it only checks the status, which comes from get_connection, before my change)

Also, test_client is also returning a 403, and that's a GET method, also failing at get_connection (and my change should only affect the HEAD method)

I may be missing something but isn't this github rate limiting or something related? If not, I'll come back for a better look

[boegel]

Could definitely be related to rate limiting, we haven't set up a GitHub token for the tests in Travis like we have for EasyBuild (but we'd also need to disable tests for PRs that require a token, since tokens are not available for testing PRs in Travis, to avoid that they leak on automatic testing of PRs).

[migueldiascosta]

@boegel @stdweird I think the rollback confirms that the problem is not with my change

also, running the tests manually with python setup.py test -F rest works for me, with and without the change, can you also test manually?

If that's the case, I can bring back my change, update the test to also check thad head() does return the headers and skip the tests if there is no token available, as in EB?

[boegel]

@migueldiascosta Can you please re-instate the fix (by just removing the rollback commit + git push --force)?

The problem is indeed that we don't have a token in place for GitHub, see also #268.

It's not trivial to put that in place though, and it won't fix the problem for PRs anyway (unless we skip tests that fail because of rate limiting).

We can work around it temporarily by retriggering the tests in Travis that broke because of rate limiting, for now (as we've done in other recent PRs).

[migueldiascosta]

@boegel done

what about also testing if head() is indeed returning the headers? something like

         """Test all request methods"""
         status, body = self.client.head()
         self.assertEqual(status, 200)
+        self.assertTrue(body)
+        self.assertIn('X-GitHub-Media-Type', body)
         try:
             status, body = self.client.user.emails.post(body='[email protected]')
             self.assertTrue(False, 'posting to unauthorized endpoint did not trhow a http error')

[boegel]

@migueldiascosta Enhancing the tests to cover the change you made makes a lot of sense, so please do!

Tests are OK now after re-triggering them, which shows the only problem is the GitHub rate limiting...

[boegel]

Green light now from Travis after a bit of patience & re-triggering tests, thanks @migueldiascosta!

[stdweird]

@migueldiascosta thanks
wrt the failing tests, i can fix the headers errors (or you can too, see "fix license headers" in https://github.com/hpcugent/vsc-install/#fix-failing-tests)
but can you have a look at the other failing test (as it looks related to the code you modified)?

[stdweird]

@migueldiascosta nvm that last remark, my firefox did someting stupid
thanks for this PR!

[migueldiascosta]

@boegel shall I submit a PR to the 4.x branch of EB?

[boegel]

@migueldiascosta Please do!