8000 Add a version check to provide good errors on version incompatibility by graysonarts · Pull Request #59 · tableau/document-api-python · GitHub
[go: up one dir, main page]

Skip to content

Add a version check to provide good errors on version incompatibility #59

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

Merged
merged 4 commits into from
Jul 26, 2016
Merged

Conversation

graysonarts
Copy link
Contributor

partial fix for #58

@benlower
Copy link
Contributor

what else needs to be done to make this a 'full' fix?

@graysonarts
Copy link
Contributor Author

I don't quite know yet. Waiting on the non-8.0 ones that are failing
On Fri, Jul 22, 2016 at 4:08 PM Ben Lower notifications@github.com wrote:

what else needs to be done to make this a 'full' fix?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#59 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFxVRKNnZdv2eDjejLMCzvfYj4ElbDVks5qYU1ggaJpZM4JTMa1
.

@t8y8
Copy link
Contributor
t8y8 commented Jul 23, 2016

Working on some stats to veirfy

@t8y8
Copy link
Contributor
t8y8 commented Jul 23, 2016

Counter({'8.0': 46, '8.2': 2, '8.1': 1, '9.0': 1})

One of the workbooks was 9.0, I'll send that one to @RussTheAerialist to see if it's related or not

@graysonarts
Copy link
Contributor Author

Thanks @t8y8 ! I'll either get a fix done this weekend or Monday (depending on how my weekend goes)

MIN_SUPPORTED_VERSION = Version("9.0")


class VersionNotSupportedException(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TableauFileVersionNotSupported or TableauVersionNotSupported?

@graysonarts
Copy link
Contributor Author

Btw, the last commit makes this a complete fix for #58

@@ -16,10 +38,10 @@ def temporary_directory(*args, **kwargs):
shutil.rmtree(d)


def find_file_in_zip(zip):
for filename in zip.namelist():
def find_file_in_zip(zip_file):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is because zip is a built-in so we should not use that as a name of the argument.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's coo' w. me

@t8y8
Copy link
Contributor
t8y8 commented Jul 25, 2016

🚀

@benlower
Copy link
Contributor

🚀

@graysonarts graysonarts merged commit aa93eef into tableau:development Jul 26, 2016
@graysonarts graysonarts deleted the 58-fix branch July 26, 2016 02:36
@graysonarts
Copy link
Contributor Author

This is the last fix before 0.2 is cut (unless someone speaks up). I'm going to cut the release in the morning.

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
0