-
Notifications
You must be signed in to change notification settings - Fork 459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keyerror data token #316
Keyerror data token #316
Conversation
I chose to return None because get_file() also returns None when response.ok is false. This actually makes detecting why get_file() failed pretty hard. Maybe get_file() should raise a PyiCloudAPIResponseException instead when response.ok is false, and a KeyError when neither data_token and package_token are found? |
bfba0c7
to
ad10ac4
Compare
Any plan to merge this PR? |
Maybe @nzapponi can approve it? |
Hi! I was also working on a code that's on the README right now, i.e.:
Then I got the |
If you try to open a |
@balloob Please consider this PR that fixed an import bug. It merges nicely with the current master. |
c595f98
to
f388787
Compare
pyicloud/services/drive.py
Outdated
else: | ||
raise KeyError("'data_token' nor 'package_token'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | |
raise KeyError("'data_token' nor 'package_token'") | |
raise PyiCloudAPIResponseException("'data_token' nor 'package_token' found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before my patch get_file
did return a KeyError
just like get_children
also does. IMHO KeyError
is the correct exception because the data_token
is missing the expected keys.
pyicloud/pyicloud/services/drive.py
Lines 242 to 253 in a14a577
def get_children(self): | |
"""Gets the node children.""" | |
if not self._children: | |
if "items" not in self.data: | |
self.data.update(self.connection.get_node_data(self.data["docwsid"])) | |
if "items" not in self.data: | |
raise KeyError("No items in folder, status: %s" % self.data["status"]) | |
self._children = [ | |
DriveNode(self.connection, item_data) | |
for item_data in self.data["items"] | |
] | |
return self._children |
Breaking change
Proposed change
Type of change
Example of code:
This produces a KeyError: 'data_token' before this patch when a packaged file like a keynote presentation is attempted to be downloaded.
Additional information
I was unable to download keynote files. Even though they have type FILE they do not contain a data_token URL. Instead they have a package_token URL. This patch tries them in that order or returns None if neither are available.
Checklist
If user exposed functionality or configuration variables are added/changed: