Deprecate Tableau personal token authentication

[samgans]

This PR closes #16669.

Why:

Authentication by personal token was not working properly since the Tableau provider's creation due to how the Tableau server client works (it invalidates previous personal token connection if one or more parallel are opened). Therefore, we decided to deprecate it.

Changes:

1. Remove _auth_via_token method from the TableauHook and make the documentation notes about the deprecation.
2. Fix get_all method of TableauHook to raise a verbose error when there is wrong object type was supplied.
3. Deprecate the usage of TableauJobStatusSensor in case of blocking TableauRefreshWorkbookOperator execution to remove the creation of duplicated connection (which is done by a sensor) and to improve the conformity of the class to SRP. Also fix some small documentation issues.
4. Deprecate testing of the personal token authentication and add change the logic of "blocked" task test.

[eladkal]

Nice analysis of the issue!

Can we deprecate the usage of personal tokens first rather than remove it completely?
I see no reason to make a breaking change here.
People who suffer from this issue will be able to migrate immediately to another authentication method while users who don't will have time to work on it till the next major release.

BTW IMHO as long as Tableau python package allows authentication with personal tokens (despite their limitation) I don't think we should prevent it. If a user wants to use them despite their limited capabilities why should we prevent it? WDYT?

[eladkal]

The place for this is in the docs:
https://github.com/apache/airflow/blob/main/docs/apache-airflow-providers-tableau/connections/tableau.rst

[samgans]

Hi, @eladkal!

The thing here is that personal token authentication was not working correctly as intended in the scope of Airflow logic, that's why we decided to remove it. But you are right, such a breaking change is what we can avoid at this stage.

Therefore, I can propose to leave the changes I've done apart from removing the method for the authentication in the hook and tests for it. I will put it back and indicate the danger of using a personal token authentication approach in the documentation and the body of the method. Does that make sense?

[eladkal]

Hi, @eladkal!

The thing here is that personal token authentication was not working correctly as intended in the scope of Airflow logic, that's why we decided to remove it. But you are right, such a breaking change is what we can avoid at this stage.

Therefore, I can propose to leave the changes I've done apart from removing the method for the authentication in the hook and tests for it. I will put it back and indicate the danger of using a personal token authentication approach in the documentation and the body of the method. Does that make sense?

So yeah we should deprecate it first to avoid breaking change. I'm not so sure about removing it as the upstream lib has this functionality in placed. It could be that they will fix the bug related to the parallelism if not we can always remove it eventually.

[potiuk]

Some static checks are failing. I HEARTILY recommend to install pre-commit (as suggested in the error message). It will then even auto-correct everything for you @samgans

[samgans]

Hello, @potiuk . The thing is that static check asks me to change this:

warnings.warn(
            "Authentication via personal access token is deprecated. "
            "Please, use the password authentication to avoid inconsistencies.",
            DeprecationWarning
        )

To this:

warnings.warn(
            "Authentication via personal access token is deprecated. "
            "Please, use the password authentication to avoid inconsistencies.",
            DeprecationWarning,
        )

Which is not the thing we should have, I guess. What do you think?

UPD: I've rechecked the PEP8 and see that the trailing comma, in this case, is not redundant if we expect this to be extended. I think I will put it here. Thanks.

[kaxil]

cc @eladkal

[eladkal]

LGTM

[github-actions]

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.