Skip to content
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

Add role parameter #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add role parameter #1

wants to merge 1 commit into from

Conversation

damian3031
Copy link
Owner

Description

Non-technical explanation

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

trino/client.py Outdated
def _format_roles(self, roles):
def _format_roles(self, role, roles):
if role and roles:
raise ValueError("specify 'role' or 'roles' parameter, but not both")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError("specify 'role' or 'roles' parameter, but not both")
raise ValueError("specify either 'role' or 'roles' parameter, but not both")

trino/dbapi.py Outdated
@@ -131,6 +132,7 @@ def __init__(
transaction_id=NO_TRANSACTION,
extra_credential=extra_credential,
client_tags=client_tags,
role=role,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to last field to not break compatiblity

@damian3031 damian3031 force-pushed the add-role-parameter branch 2 times, most recently from e8a6445 to e886aba Compare March 2, 2023 19:28
@damian3031 damian3031 force-pushed the add-role-parameter branch from e886aba to 1319a18 Compare April 3, 2023 09:49
README.md Outdated
host='localhost',
port=443,
user='the-user',
roles="roleC" # equivalent to {"system": "roleC"}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid mixing uppercase and lowercase in the example.

@damian3031 damian3031 force-pushed the add-role-parameter branch from 1319a18 to 026e36f Compare April 3, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants