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

make CLI authentication case-insensitive #100

Closed
dataders opened this issue Feb 5, 2021 · 2 comments · Fixed by #113
Closed

make CLI authentication case-insensitive #100

dataders opened this issue Feb 5, 2021 · 2 comments · Fixed by #113

Comments

@dataders
Copy link
Collaborator

dataders commented Feb 5, 2021

in #99, we found out that users assume that authentication: CLI can also be given as authentication: cli which is not the case.

@JCZuurmond... if you're interested 👀

Spitball idea, but maybe we add an .upper() to the this snippet below. something like:

if type_auth.upper() in AZURE_AUTH_FUNCTIONS.keys()

if type_auth in AZURE_AUTH_FUNCTIONS.keys():
# create token if it does not exist
if cls.TOKEN is None:
azure_auth_function = AZURE_AUTH_FUNCTIONS[type_auth]
token = azure_auth_function(credentials)
cls.TOKEN = convert_access_token_to_mswindows_byte_string(
token
)

@JCZuurmond
Copy link
Contributor

@swanderz : That would indeed solve the problem for when authentication type is cli instead of CLI (or cLi for that matter). However it would not make the other authentication types case insensitive.

I have the urge to restructure some code first so that we can add (py)tests. It would make me more confident about changing code without breaking it.

However, if we prefer to do a quick fix first, then that is ok with me.

@dataders
Copy link
Collaborator Author

dataders commented Feb 5, 2021

no urgency to make a hotfix, given that this issue is documented and easily fixed. more tests would be great -- thanks for offering. Happy to add any tests you write to CircleCI.

JCZuurmond added a commit to JCZuurmond/dbt-sqlserver that referenced this issue Apr 14, 2021
JCZuurmond added a commit to JCZuurmond/dbt-sqlserver that referenced this issue Apr 14, 2021
JCZuurmond added a commit to JCZuurmond/dbt-sqlserver that referenced this issue Apr 14, 2021
JCZuurmond added a commit to JCZuurmond/dbt-sqlserver that referenced this issue Apr 14, 2021
jeroen-mostert pushed a commit to jeroen-mostert/dbt-sqlserver that referenced this issue Jun 27, 2021
jeroen-mostert pushed a commit to jeroen-mostert/dbt-sqlserver that referenced this issue Jun 27, 2021
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 a pull request may close this issue.

2 participants