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 support for access tokens #185

Merged
merged 5 commits into from
May 6, 2022
Merged

Conversation

kimjamia
Copy link
Contributor

Add support for login in with an access token to SQL Server / Azure SQL Database.

Required for #115 as tokens can't be passed via the connection string.

@kimjamia kimjamia changed the title Feat/add access token Add support for access tokens Apr 11, 2022
@wokket
Copy link
Collaborator

wokket commented Apr 28, 2022

@erikbra We look to have some CI issues atm, I'll try and have a look at those tonight for you.

@kimjamia
Copy link
Contributor Author

I think I had that same issue locally (before my change) but I figured I messed up something so I just commented it out to get it to build but didn't commit it.

@wokket
Copy link
Collaborator

wokket commented May 4, 2022

@kimjamia Sorry for the delay here, I know @erikbra is really busy.

I've fixed the build issues (caused by breaking changes upstream), could I get you to rebsae/pull the latest into your branch so it builds and we'll get this sorted?

@kimjamia
Copy link
Contributor Author

kimjamia commented May 5, 2022

I tested that unit tests pass and my change also still works after merging main to my branch.

@wokket
Copy link
Collaborator

wokket commented May 5, 2022

Yep all looks good on this side. I was just giving Erik another day or two to respond (as it is primarily his codebase)

@erikbra
Copy link
Owner

erikbra commented May 6, 2022

Hi, guys. Terribly sorry for going "missing" on you here. There are a lot of other things going on ATM, so I haven't had much (any) time to follow up here.

I really, really wish the SqlConnection could support specifying the access token in the connection string, but, apparently it doesn't. It was definitely an oversight on my part originally.

Thanks a lot for your contribution, @kimjamia ! And thanks for following up, @wokket !

Merging this :)

@erikbra erikbra merged commit e84c60e into erikbra:main May 6, 2022
@kimjamia kimjamia deleted the feat/add-access-token branch May 11, 2022 18:03
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.

3 participants