Added JWT authentication to the TableauHook #51756

dominikhei · 2025-06-15T11:36:59Z

I added JWT Authentication to the TableauHook. If password and login are set, it is preferred over JWT Authentication.
There are two options for using a token: Passing it as a string to the connection extras or using a path to a location on disk to a file. If both are set, the file will be used.
closes: #51453

eladkal · 2025-06-15T12:04:27Z

You probably need to run
breeze static-checks --type update-providers-dependencies --all-files

dominikhei · 2025-06-15T13:37:55Z

@eladkal The problem is between apache-airflow-providers-amazon[aiobotocore] and apache-airflow-providers-tableau:

botocore requires urllib3 versions between 1.25.4 and 1.27 for Python < 3.10
tableauserverclient>=0.27 (which we need for the JWT authentication) requires urllib3>=2.0.6

It seems like this issue is not a problem with Python>=3.10 only with < 3.10

eladkal · 2025-06-15T14:24:37Z

I think we can set the provider to be Python 3.10+ only. Python 3.9 is EOL very soon and anyway we don't introduce that many changes to the tableau provider.
cc @potiuk WDYT?

dominikhei · 2025-06-16T21:04:17Z

dependency issues seem to be resolved with python3.10 when testing locally. However breeze static-checks --type update-providers-dependencies --all-files always uses a python3.9 ci-image, no matter what I try to change.

eladkal · 2025-06-16T22:27:40Z

Can you push the changes? Lets see
Probably need to set min Python version for the provider, not sure if we allow this per provider. Lets see what the CI says.
We can also limit usage of the new feature for Python 3.10+ if needed without changing the min Python vesrion.

eladkal · 2025-06-17T12:46:26Z

@dominikhei try to run:
breeze ci-image build --upgrade-to-newer-dependencies

dominikhei · 2025-06-17T14:40:50Z

@dominikhei try to run: breeze ci-image build --upgrade-to-newer-dependencies

Already tried it, still tries to run the checks with the 3.9 ci-image. maybe its hardcoded somewhere because I can not find a way to use the 3.10 image, even though I have configured breeze to use it.

eladkal · 2025-06-19T17:59:29Z

@dominikhei try to run: breeze ci-image build --upgrade-to-newer-dependencies

Already tried it, still tries to run the checks with the 3.9 ci-image. maybe its hardcoded somewhere because I can not find a way to use the 3.10 image, even though I have configured breeze to use it.

We have a case of excluded Python 3.9 from a provider. apply similar logic as shown here:

airflow/providers/cloudant/pyproject.toml

Lines 52 to 63 in a4a51a0

    
           requires-python = "~=3.9,!=3.9" 
        
           # The dependencies should be modified in place in the generated file. 
        
           # Any change in the dependencies is preserved when the file is regenerated 
        
           # Make sure to run ``breeze static-checks --type update-providers-dependencies --all-files`` 
        
           # After you modify the dependencies, and rebuild your Breeze CI image with ``breeze ci-image build`` 
        
           dependencies = [ 
        
               "apache-airflow>=2.10.0", 
        
               # Even though 3.9 is excluded below, we need to make this python_version aware so that `uv` can generate a 
        
               # full lock file when building lock file from provider sources 
        
               "ibmcloudant==0.9.1;python_version>=\"3.10\"", 
        
           ]

dominikhei · 2025-06-19T19:08:19Z

Apart from the failing doc checks, which i'll adjust, the CI still runs with 3.9 leading to tableauserverclient now not being included as a dependency (tableauserverclient>=0.27;python_version>=\"3.10\"). Locally the breeze static-checks --type update-providers-dependencies --all-files worked.

eladkal · 2025-06-20T11:42:15Z

Looks like we need also to make changes in global_constants.py add tableau to all Python 3.9 entries in remove-providers:

airflow/dev/breeze/src/airflow_breeze/global_constants.py

Lines 725 to 744 in 96c48a5

    
           PROVIDERS_COMPATIBILITY_TESTS_MATRIX: list[dict[str, str | list[str]]] = [ 
        
               { 
        
                   "python-version": "3.9", 
        
                   "airflow-version": "2.10.5", 
        
                   "remove-providers": "cloudant common.messaging fab git keycloak", 
        
                   "run-tests": "true", 
        
               }, 
        
               { 
        
                   "python-version": "3.9", 
        
                   "airflow-version": "2.11.0", 
        
                   "remove-providers": "cloudant common.messaging fab git keycloak", 
        
                   "run-tests": "true", 
        
               }, 
        
               { 
        
                   "python-version": "3.9", 
        
                   "airflow-version": "3.0.2", 
        
                   "remove-providers": "cloudant", 
        
                   "run-tests": "true", 
        
               }, 
        
           ]

might also need change in test-providers.yaml but not 100% sure, lets apply the above fix first and see if we need to change here too

airflow/.github/workflows/test-providers.yml

Lines 129 to 132 in 3198aad

    
                 - name: Remove Python 3.9-incompatible provider distributions 
        
                   run: | 
        
                     echo "Removing Python 3.9-incompatible provider: cloudant" 
        
                     rm -vf dist/*cloudant*

We just need to read the errors to get direction of what is the issue and then look up for cloudant key word in the code base to see what special treatment it gets. The issue of excluding Python version in a specific provider is not common thus we don't have 1 line fix for this yet.

dominikhei · 2025-06-20T12:24:46Z

Might be helpful for later once we made it work, to also adjust the Contributors Guide with a section on this.

eladkal · 2025-06-20T12:33:00Z

airflow-core/tests/unit/always/test_providers_manager.py:256: in test_hook_values
    raise AssertionError("There are warnings generated during hook imports. Please fix them")
E   AssertionError: There are warnings generated during hook imports. Please fix them

failure is because we are missing similar:

airflow/providers/cloudant/provider.yaml

Lines 57 to 62 in a4a51a0

    
           excluded-python-versions: 
        
             # ibmcloudant transitively brings in urllib3 2.x, but the snowflake provider has a dependency that pins 
        
             # urllib3 to 1.x on Python 3.9; thus we exclude those Python versions from taking the update 
        
             # to ibmcloudant. 
        
             # See #21004, #41555, and https://github.com/snowflakedb/snowflake-connector-python/issues/2016 
        
             - "3.9"

eladkal · 2025-06-20T12:36:30Z

Might be helpful for later once we made it work, to also adjust the Contributors Guide with a section on this.

You will be the best person for the job. Ideally, if we can find automations I think it's better.
we already have the excluded-python-versions per provider so I think it would be better to try to automate the tests to account for this

eladkal · 2025-06-20T13:05:23Z

OK looks like all cutrent failures are docs/examples and tableau tests

dominikhei · 2025-06-20T13:13:58Z

OK looks like all cutrent failures are docs/examples and tableau tests

Seems like airflow/providers/tableau/docs/tableau.rstwas not previously tracked and I added it by accident

The failing tests are still linked to the python version:

_ ERROR collecting providers/tableau/tests/unit/tableau/sensors/test_tableau.py _
ModuleNotFoundError: No module named 'tableauserverclient'

eladkal · 2025-06-20T13:40:56Z

can you fix the other tests? Sometimes handling the other errors solves the doc problems

eladkal · 2025-06-20T16:33:36Z

Almost :)
We need to skip tests on Python 3.9 runners - this needs to be done on all Tableau test files

airflow/providers/cloudant/tests/unit/cloudant/hooks/test_cloudant.py

Lines 30 to 37 in 4d5846f

    
           if sys.version_info < (3, 10): 
        
               pytestmark.append( 
        
                   pytest.mark.skip( 
        
                       f"Skipping {__name__} as the cloudant provider is not supported on Python 3.9, see #41555." 
        
                   ) 
        
               ) 
        
           else: 
        
               from airflow.providers.cloudant.hooks.cloudant import CloudantHook

dominikhei · 2025-06-22T04:55:55Z

@eladkal the only failing static check is ts-compile-lint-ui due to some code quality violations in the frontend code. I just checked and the code responsible for one of the errors was last touched on 22.04.2025.

Do you know why this is failing now?

gopidesupavan · 2025-06-22T09:10:20Z

@eladkal the only failing static check is ts-compile-lint-ui due to some code quality violations in the frontend code. I just checked and the code responsible for one of the errors was last touched on 22.04.2025.

Do you know why this is failing now?

we reverted dependaboat bumps, please rebase it should work now?

eladkal · 2025-06-22T09:59:23Z

I started [DISCUSS] Dropping Python 3.9 support
lets see if we can get it passed soon thus making this PR easier

dominikhei · 2025-06-22T15:16:08Z

I started [DISCUSS] Dropping Python 3.9 support lets see if we can get it passed soon thus making this PR easier

Should we just wait until we upgrade to 3.10, as the feedback was positive or add it for 3.9 with the workarounds? (Would need to rebase due to #51930)

eladkal · 2025-06-27T13:19:12Z

#52072 is merged. You can now proceed without the trouble of Python 3.9

eladkal · 2025-06-30T03:37:54Z

providers/tableau/docs/connections/tableau.rst

+   i.e add a ``jwt_file`` or a ``jwt_token`` to the Airflow connection extras.
+
+If both Password and Username authentication and JWT authentication are used simultaneously,
+Password and Username authentication is preferred.


How can both be allowed at the same time?
Isn't it better to raise mutually exclusive exception?

My point of view was that as long as we document the behavior and only one method is used, there is no harm in allowing credentials for both to be included at the same time. But I'll adjust it, as one second thought there is more speaking against than for it.

@eladkal I’ve updated the behavior to raise an error when both authentication methods are configured. However, the Trino provider allows the same behavior:
Jwt and password auth can be set simultaneously, but password is silently preferred. Moreover it raises an error if cert / kerberos auth and a password is set.

For consistency, should we consider updating it as well?

eladkal force-pushed the tableau-jwt branch from 4deca24 to 1ff2bd3 Compare June 20, 2025 11:17

dominikhei force-pushed the tableau-jwt branch from c0c0714 to 02e1a2d Compare June 20, 2025 14:29

dominikhei force-pushed the tableau-jwt branch from d649e27 to cf584fb Compare June 22, 2025 13:17

dominikhei added 3 commits June 29, 2025 18:41

Added jwt authentication to the TableauHook

74aa988

Added further unit tests

dc54ffd

Adjusted Tableau connection docs

4524762

dominikhei force-pushed the tableau-jwt branch from d5b02ee to 4524762 Compare June 29, 2025 18:13

dominikhei marked this pull request as ready for review June 29, 2025 21:46

eladkal reviewed Jun 30, 2025

View reviewed changes

Added mutually exclusive check for auth methods

4a3cc5f

dominikhei requested a review from eladkal June 30, 2025 13:11

Added JWT authentication to the TableauHook #51756

Are you sure you want to change the base?

Added JWT authentication to the TableauHook #51756

Uh oh!

Conversation

dominikhei commented Jun 15, 2025 • edited by eladkal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 15, 2025

Uh oh!

dominikhei commented Jun 15, 2025

Uh oh!

eladkal commented Jun 15, 2025

Uh oh!

dominikhei commented Jun 16, 2025

Uh oh!

eladkal commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 17, 2025

Uh oh!

dominikhei commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 19, 2025

Uh oh!

dominikhei commented Jun 19, 2025

Uh oh!

eladkal commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dominikhei commented Jun 20, 2025

Uh oh!

eladkal commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 20, 2025

Uh oh!

eladkal commented Jun 20, 2025

Uh oh!

dominikhei commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 20, 2025

Uh oh!

eladkal commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dominikhei commented Jun 22, 2025

Uh oh!

gopidesupavan commented Jun 22, 2025

Uh oh!

eladkal commented Jun 22, 2025

Uh oh!

dominikhei commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eladkal commented Jun 27, 2025

Uh oh!

eladkal Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

dominikhei Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dominikhei Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dominikhei commented Jun 15, 2025 •

edited by eladkal

Loading

eladkal commented Jun 16, 2025 •

edited

Loading

dominikhei commented Jun 17, 2025 •

edited

Loading

eladkal commented Jun 20, 2025 •

edited

Loading

eladkal commented Jun 20, 2025 •

edited

Loading

dominikhei commented Jun 20, 2025 •

edited

Loading

eladkal commented Jun 20, 2025 •

edited

Loading

dominikhei commented Jun 22, 2025 •

edited

Loading

dominikhei Jun 30, 2025 •

edited

Loading