Skip to content

Conversation

@madatx
Copy link
Contributor

@madatx madatx commented May 12, 2020

Hi,

I've tried to use ClientV1.download_order to download all locations at once and got exception:

# ...
File "/usr/local/lib/python3.7/site-packages/planet/api/client.py", line 568, in download_order
  return self._get(locations, models.JSON, callback=callback)
File "/usr/local/lib/python3.7/site-packages/planet/api/client.py", line 61, in _get
  request = self._request(path, body_type, params)
File "/usr/local/lib/python3.7/site-packages/planet/api/client.py", line 52, in _request
  return models.Request(self._url(path), auth or self.auth, params,
File "/usr/local/lib/python3.7/site-packages/planet/api/client.py", line 45, in _url
  if path.startswith('http'):
AttributeError: 'list' object has no attribute 'startswith'

Looks like the problem is that client _get method expects a single location but gets a list instead.

This PR is a possible solution.

@strixcuriosus
Copy link
Contributor

Thanks for the contribution! Your fix looks good to me.

@strixcuriosus
Copy link
Contributor

The build failure is not specific to your branch. I made a separate PR with a fix for these lint and docs errors: #211

@strixcuriosus
Copy link
Contributor

I would recommend adding a test case for the bug you discovered, but the orders methods seem to lack test coverage in general. I've created this issue to add test coverage to the orders related ClientV1 methods #212

@strixcuriosus strixcuriosus merged commit 7c2a416 into planetlabs:master May 13, 2020
@strixcuriosus
Copy link
Contributor

@sarasafavi I see that you've created most of the recent releases. Do you have any concerns about creating a new release to get this orders fix in?

@sarasafavi
Copy link
Contributor

@strixcuriosus the next package release is already lined up and in testing (and will include those text fixes referenced in #211 ), but I can definitely get this fix wrapped into the next next release.

Thank you, @madatx, for finding and fixing this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants