Skip to content

It's not possible to set role in trino.dbapi.connect method #230

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

Closed
1 task done
aksakalli opened this issue Sep 2, 2022 · 1 comment · Fixed by #212
Closed
1 task done

It's not possible to set role in trino.dbapi.connect method #230

aksakalli opened this issue Sep 2, 2022 · 1 comment · Fixed by #212

Comments

@aksakalli
Copy link
Member

aksakalli commented Sep 2, 2022

Describe the feature

With the recent changes with #200 X-Trino-Role and X-Trino-Set-Role are not allowed to be set. The following code:

connection = trino.dbapi.connect(
    host="trino.example.net",
    port=443,
    http_scheme="https",
    catalog="hive",
    user='abuzer',
    auth=trino.auth.JWTAuthentication('Xvfdas.....'),
    http_headers={'X-Trino-Role': 'hive=ROLE{admin}'}
)

gives an error ValueError: cannot override reserved HTTP header X-Trino-Role.

We should make the role parameter part of trino.dbapi.connect method.

Describe alternatives you've considered

I can to set role in connection._client_session.role or connection._http_session.headers. Since _client_session property should be an internal abstraction, connection._client_session.role='hive=ROLE{admin}' is ugly and prone to change. We need a proper interface to set such values in connect method.

We should keep release notes and publish critical breaking changes in future.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@mdesmet
Copy link
Contributor

mdesmet commented Sep 5, 2022

Hi @aksakalli,

There is already an open PR for this: See #212

Note that the current implementation is not tested with multiple catalogs. I tried adding a test:

@pytest.mark.skipif(trino_version() == '351', reason="Newer Trino versions return the system role")
def test_set_multiple_roles_trino_351(run_trino):
    _, host, port = run_trino

    trino_connection = trino.dbapi.Connection(
        host=host, port=port, user="test", catalog="tpch"
    )
    cur = trino_connection.cursor()
    cur.execute('SHOW TABLES FROM information_schema')
    cur.fetchall()
    assert cur._request._client_session.role is None

    cur.execute("SET ROLE ALL")
    cur.fetchall()
    assert cur._request._client_session.role == "system=ALL"

    cur.execute("SET ROLE ALL IN tpch")
    cur.fetchall()
    assert cur._request._client_session.role == "system=ALL;tpch=ALL"

However we need to add the hive connector as tpch or memory doesn't support role management.

You could also try to do cur.execute("SET ROLE admin in hive") as a first query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants