Don't hard code expecting HTTP 200 as the only success response code, all 20x responses are success codes.

[asherf]

Problem

Pants treats non-200 HTTP responses from the server as errors. 20x HTTP responses should not be considered errors.

Solution

Use the request's library built in response.ok property to determine if we got an HTTP error response or not.

Result

HTTP 20x responses are handled is valid success responses.

[asherf]

calling out the original change:
e0248e0#diff-0f1ccfaf25380a7a992f3bb651319718R231

in case @benjyw wants to clarify something.
@jsirois too

[Eric-Arellano]

Thanks!

[jsirois]

This all might be a bit less awkward if the 1st check was for ok, ie:

if res.ok:
  return True
elif res.status_code in {307, 308}:
  return do_post(res.headers['location'], num_redirects_allowed - 1)
else:
  error(f'HTTP error code: {res.status_code}. Reason: {res.reason}.')
  if 300 <= res.status_code < 400 or res.status_code == 401:
    print(f'Use `path/to/pants login --to={auth_provider}` to authenticate '
          'against the stats upload service.', file=sys.stderr)
  return False

[asherf]

@jsirois this won't work, since 307 and 307 are also considered ok responses, and we want to treat those as errors.
https://github.com/psf/requests/blob/master/requests/models.py#L702
https://github.com/psf/requests/blob/a4c18cd733f97b5659a29589432d8a39e7a0de87/requests/models.py#L933-L936

[asherf]

There is actually a test to validate that HTTP 307 results in an error:
https://github.com/pantsbuild/pants/pull/8102/files#diff-9e6ae65d103fa02ecc6e5f0ce5eeb3d7L57

[jsirois]

@asherf the error in the 'OSX platform-specific tests (Py3.6 PEX)' shard are persistent: https://travis-ci.org/pantsbuild/pants/jobs/567969022:

...
16:51:56 07:15       [run]
                     ...F....                                   [100%]
                     ==================== FAILURES ====================
                     _ ConanFetchTest.test_rewrites_remotes_according_to_options _
                     
                     self = <pants_test.backend.native.tasks.test_conan_fetch.ConanFetchTest testMethod=test_rewrites_remotes_according_to_options>
                     
                         def test_rewrites_remotes_according_to_options(self):
                           self.set_options(conan_remotes={'pants-conan': 'https://conan.bintray.com'})
                           conan_prep_task_type = self.synthesize_task_subtype(ConanPrep, 'conan_prep_scope')
                           # We need at least one library to resolve here so that the conan pex is generated.
                           dummy_target = self.make_target(spec='//:dummy-conan-3rdparty-lib',
                                                           target_type=ExternalNativeLibrary,
                                                           packages=[])
                           context = self.context(for_task_types=[conan_prep_task_type],
                                                  target_roots=[dummy_target])
                           conan_prep = conan_prep_task_type(context, os.path.join(self.pants_workdir, 'conan_prep'))
                           conan_fetch = self.create_task(context, os.path.join(self.pants_workdir, 'conan_fetch'))
                           conan_prep.execute()
                     >     conan_fetch.execute()
                     
                     .pants.d/pyprep/sources/9e124fc75cacc56751e0dafdc453479d36ca9db9/pants_test/backend/native/tasks/test_conan_fetch.py:38: 
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     .pants.d/pyprep/sources/9e124fc75cacc56751e0dafdc453479d36ca9db9/pants/task/simple_codegen_task.py:226: in execute
                         sources = self._capture_sources((vt,))[0]
                     .pants.d/pyprep/sources/9e124fc75cacc56751e0dafdc453479d36ca9db9/pants/task/simple_codegen_task.py:296: in _capture_sources
                         snapshots = self.context._scheduler.capture_snapshots(tuple(to_capture))
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     
                     self = <pants.engine.scheduler.SchedulerSession object at 0x1b919f6ec50>
                     path_globs_and_roots = (PathGlobsAndRoot(path_globs=PathGlobs(include=('.pants.d/conan_fetch/f8134db94d8d/.dummy-conan-3rdparty-lib/current/i...y_match')), root='/private/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/tmpwg9rpnkj_BUILD_ROOT', digest_hint=None),)
                     
                         def capture_snapshots(self, path_globs_and_roots):
                           """Synchronously captures Snapshots for each matching PathGlobs rooted at a its root directory.
                         
                             This is a blocking operation, and should be avoided where possible.
                         
                             :param path_globs_and_roots tuple<PathGlobsAndRoot>: The PathGlobs to capture, and the root
                                    directory relative to which each should be captured.
                             :returns: A tuple of Snapshots.
                             """
                           result = self._scheduler._native.lib.capture_snapshots(
                             self._scheduler._scheduler,
                             self._session,
                     >       self._scheduler._to_value(_PathGlobsAndRootCollection(path_globs_and_roots)),
                           )
                     E     TypeError: capture_snapshots expected 2 arguments, got 3
                     
                     .pants.d/pyprep/sources/9e124fc75cacc56751e0dafdc453479d36ca9db9/pants/engine/scheduler.py:534: TypeError
                     -------------- Captured stderr call --------------
                     
                     Starting workunit create-conan-pex
                     
                     Starting workunit execute
                     - generated xml file: /Users/travis/build/pantsbuild/pants/.pants.d/test/pytest/tests.python.pants_test.backend.native.tasks.tasks/junitxml/TEST-tests.python.pants_test.backend.native.tasks.tasks.xml -
                     ============ short test summary info =============
                     FAIL tests/python/pants_test/backend/native/tasks/test_conan_fetch.py::ConanFetchTest::test_rewrites_remotes_according_to_options
                     1 failed, 7 passed in 130.09 seconds
...

The API complained about here is correct on master:

result = self._scheduler._native.lib.capture_snapshots(
  self._scheduler._scheduler,
  self._session,
  self._scheduler._to_value(_PathGlobsAndRootCollection(path_globs_and_roots)),
)

I tried nuking Travis caches for your branch though and this persists. Perhaps try re-basing your branch against master to kick off a new CI run. I've just freshly nuked Travis caches for your PR.

[jsirois]

Yay - CI green. I'll merge post haste.

[asherf]

yay!!!