search

tableau / server-client-python

A Python library for the Tableau Server REST API
https://tableau.github.io/server-client-python/
MIT License
659 stars 421 forks source link

Issue 1299 #1300

Closed kykrueger closed 10 months ago

kykrueger commented 1 year ago

closes #1299

Fixes issue with bad state of async_response. Fixes issue that timeouts are not triggered when set to longer than 60s.

salesforce-cla[bot] commented 1 year ago

Thanks for the contribution! Before we can merge this, we need @kykrueger to sign the Salesforce Inc. Contributor License Agreement.

kykrueger commented 1 year ago

I've signed the CLA

bcantoni commented 1 year ago

Close and reopen PR to run CLA check again.

bcantoni commented 1 year ago

@kykrueger thanks for putting this PR together! As a first time contributor we had to click a button to run all the checks here and those are done now. You are all set from the CLA perspective. If you look at the build results it looks like you have one black formatting issue and one by pi type issue to fix.

In parallel, @jacalata could you give this a review?

kykrueger commented 1 year ago

Alright, I'll run black over it once. Are you using the defaults or the --preview flag?

jacalata commented 1 year ago

Defaults

kykrueger commented 1 year ago

@jacalata @bcantoni that should do it

kykrueger commented 1 year ago

@jacalata @bcantoni the mypy failures are not functionally related to the PR. Do you have a suggestion?

kykrueger commented 1 year ago

To push this along I've added a check to see if exceptions are returned from the blocking request. This was already returned before this PR, but the type hinting didn't reveal the possibility.

When making my changes, I fixed a pyright error where it had complained that Exceptions could be returned, but were not hinted. This broke the checks for mypy.

Now the returned exceptions will be raised. Before a typeerror would have probably occurred, but now the proper exception will be raised.

kykrueger commented 1 year ago

@bcantoni @jacalata could I get a review and restart of the builds?

kykrueger commented 1 year ago

Thanks!

kykrueger commented 11 months ago

@jacalata , I adjusted the settings of my config for Black to match those of your tests. Now that we both have 120 as the line length, I expect all builds to pass.

kykrueger commented 11 months ago

@bcantoni @jacalata could you give an Update on what is holding up this ticket? FYI: I just rebased on main

mzappitello commented 11 months ago

is there any update on this PR? our workflows ran into the same error described by @kykrueger and we had to revert back to v0.25.