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 Task overload of constructor to external provider to allow it to run async #1269

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

quails4Eva
Copy link
Contributor

@quails4Eva quails4Eva commented Sep 9, 2023

Allow passing a Task to the constructor so auth can be done async. Some of the complexity is to ensure it's backwards compatible with existing code.

I don't think the tests are quite right yet, would appreciate some guidance on how best to handle it for this project. Simplest thing to do would be to copy the existing ones but just use the new constructor.

A few things I was thinking about.

  • Do the tests actually need to connect to Sharepoint? Would unit tests be sufficient? Perhaps this would be inconsistent with the other areas of the project.
  • Constructor could take an interface rather than a Func?
  • No use of mocking library? Not sure if you can actually mock a Func.
  • In tests could use data attribute to control which constructor to use and whether to use fake version of token provider. Possibly a bit overkill.

@jansenbe jansenbe self-assigned this Sep 20, 2023
@jansenbe
Copy link
Contributor

@quails4Eva : thanks for looking into this, being able to invoke the external access token retrieval code asynchronous does make sense.

jansenbe added a commit that referenced this pull request Sep 20, 2023
@jansenbe jansenbe merged commit 8bfa601 into pnp:dev Sep 20, 2023
@quails4Eva quails4Eva deleted the add-async-to-external-provider branch September 23, 2023 11:13
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