hydroshare / hs_restclient

Python client for the https://www.hydroshare.org REST API
BSD 3-Clause "New" or "Revised" License
6 stars 8 forks source link

Python 3: tests all pass except sometimes randomly see an odd error #27

Closed mt-digital closed 8 years ago

mt-digital commented 8 years ago

Should fix #9 and #12. basestring has been removed in Python 3, with just str to replace it (see What's New in Python 3 for details).

I ran the tests and also found a problem with the format of downloaded content in Python 3, which has been fixed here.

For a download, I feel the content should still be binary (see line 138 in hs_restclient/__init__.py). But in the case of a request for science metadata, this should explicitly be a string (see line 468 in hs_restclient/__init__.py). Python 3 is more specific when dealing with regular strings vs binary strings.

The irregular error is the following:

(venv) Jul 08 01:04:25 AM (python3) ✗ tests: python test_hs_restclient.py
.....E....
======================================================================
ERROR: test_get_resource_list_filter_date (__main__.TestGetResourceList)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mturner/workspace/hs_restclient/venv/lib/python3.5/site-packages/httmock.py", line 211, in inner
    return func(*args, **kwargs)
  File "test_hs_restclient.py", line 96, in test_get_resource_list_filter_date
    for (i, r) in enumerate(res_list):
  File "/Users/mturner/workspace/hs_restclient/hs_restclient/__init__.py", line 254, in _getResultsListGenerator
    raise HydroShareNotFound((url,))
hs_restclient.HydroShareNotFound: Resource 'https://www.hydroshare.org/hsapi/resourceList/' was not found.

----------------------------------------------------------------------
Ran 10 tests in 0.268s

FAILED (errors=1)

Many times I see total success in the tests, but the above error pops up seemingly randomly. I tried including time.sleep(...) in various places to see if the httmocks needed more time to load up. I think it has something to do with lazy generators in Python 3, but I don't know. In any case, this might be good enough to call this Python 3-compatible. Maybe you'd see how to fix it quick. If not, let me know and I will look into it more to try to get it to pass every time.

Tests pass on Python 2 with no sign of this intermittent error in the tests.

pkdash commented 8 years ago

@shawncrawley Can you review this?

shawncrawley commented 8 years ago

+1

pkdash commented 8 years ago

@hyi and @mjstealey This PR is based on the master branch. Should merge it? I am not sure what it does to the readthedocs site (the documentation site for hs_restclient) when you commit to the master.

Castronova commented 8 years ago

+1

pkdash commented 8 years ago

@hyi and @mjstealey This is a small change based of off master. I plan to merge it to master soon unless you have concerns. In the future we have to watch out that PRs are created to be merged with 'develop' branch only.