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.

Sign up for GitHub

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

Jump to bottom
Merged
graysonarts merged 4 commits into from
Jul 26, 2016
Merged

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

graysonarts merged 4 commits into from
Jul 26, 2016

Conversation

@graysonarts
Copy link
Contributor

@graysonarts graysonarts commented Jul 22, 2016

partial fix for #58

@benlower
Copy link
Contributor

benlower commented Jul 22, 2016

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

@graysonarts
Copy link
Contributor Author

graysonarts commented Jul 22, 2016

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 [email protected] 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

graysonarts commented Jul 23, 2016

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

t8y8
t8y8 reviewed Jul 23, 2016
View reviewed changes
tableaudocumentapi/xml_open.py Outdated
MIN_SUPPORTED_VERSION = Version("9.0")


class VersionNotSupportedException(Exception):
Copy link
Contributor

@t8y8 t8y8 Jul 23, 2016

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

graysonarts commented Jul 25, 2016

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

graysonarts
graysonarts reviewed Jul 25, 2016
View reviewed changes
tableaudocumentapi/xfile.py

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

@graysonarts graysonarts Jul 25, 2016

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

@t8y8 t8y8 Jul 25, 2016

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

benlower commented Jul 26, 2016

🚀

@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

graysonarts commented Jul 26, 2016

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

@graysonarts graysonarts mentioned this pull request Jul 26, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@graysonarts @benlower @t8y8