-
Notifications
You must be signed in to change notification settings - Fork 8
Unification api and tokens #54
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
Conversation
…ready exists in Test-TokenExpired cmdlet) #18
Signed-off-by: Rob Sewell <mrrobsewell@outlook.com>
Oof a lot to go through here!! Please can you include in the PR what is being changed to help reviewers to be able to assist :-) |
Linux Test Results4 567 tests 4 566 ✅ 43s ⏱️ Results for commit 3084bd8. ♻️ This comment has been updated with latest results. |
WinPS51 Test Results4 567 tests 4 566 ✅ 52s ⏱️ Results for commit 3084bd8. ♻️ This comment has been updated with latest results. |
WinPS71 Test Results4 567 tests 4 566 ✅ 52s ⏱️ Results for commit 3084bd8. ♻️ This comment has been updated with latest results. |
To my eyes it looks ok. Thank you for all the hard work so far. There are a couple of changes required to make the Pester Tests pass. You always need a CommandName.Tests.ps1 file in unit tests You need to pass the ScriptAnalyser rules You can check these with
You can always run the tests as the PR runs locally with
We need to get a load of this into the Contributing files to help everyone with these things |
Signed-off-by: Kamil Nowinski <kamil@nowinski.net>
…stMethodExtended`
…Expired` using `EnableTokenRefresh` Feature Flag - Removed `Set-FabricApiHeaders` and merged the entire logic to `Connect-FabricAccount` #44
…parameters for Pester tests
Cosmetic changes Co-authored-by: Jess Pomfret <jpomfret7@gmail.com> Signed-off-by: Kamil Nowinski <kamil@nowinski.net>
Thanks @jpomfret for your time and efforts reviewing this big PR! Much appreciate! |
Co-authored-by: Jess Pomfret <jpomfret7@gmail.com> Signed-off-by: Kamil Nowinski <kamil@nowinski.net>
@NowinskiK - I've been through all the files and added a few more comments - once these are resolved I'm happy to approve this and get it in. Thanks for all the hard work on this! (Going forward it'll be best to try and chunk PRs into smaller more manageable ones - but I know we have some overarching changes that need to touch a lot) |
Fixed Get-FabricWorkspaceUsers function
@NowinskiK amazing work in this PR. |
@jpomfret thanks for all your comments, efforts and time. I know it was a big PR and we should avoid such huge changes moving forward. |
Pull Request
Pull Request (PR) description
This pull request introduces a range of updates across multiple files, focusing on token management improvements, module version updates, and workflow enhancements. Key changes include replacing outdated authentication methods with a streamlined token refresh mechanism, updating dependencies to newer versions, and refining development and testing workflows for better clarity and efficiency.
Token Management Improvements:
Set-FabricAuthToken
withUpdate-FabricToken
for token updates and streamlined user context changes usingConnect-FabricAccount
. (README.md
README.mdL69-R74)Confirm-FabricAuthToken
and extendedTest-TokenExpired
to support automatic token refresh using theEnableTokenRefresh
feature flag. (CHANGELOG.md
[1]source/Private/Test-TokenExpired.ps1
[2] [3] [4] [5]Confirm-FabricAuthToken
calls withTest-TokenExpired
for consistent token validation. (source/Public/Capacity/*.ps1
[1] [2] [3] [4] [5] [6] [7]Dependency Updates:
Az.Accounts
dependency from version4.2.0
to5.0.0
in bothRequiredModules.psd1
andsource/FabricTools.psd1
. (RequiredModules.psd1
[1]source/FabricTools.psd1
[2]HelpInfoURI
tosource/FabricTools.psd1
for improved documentation access. (source/FabricTools.psd1
source/FabricTools.psd1L134-L140)Workflow Enhancements:
runs-on
configuration in.github/workflows/pr.yml
to usewindows-latest
instead ofwindows-2019
for testing stages. (.github/workflows/pr.yml
[1] [2]CONTRIBUTING.md
to emphasize using clean PowerShell sessions and dependency resolution. (CONTRIBUTING.md
[1] [2]Codebase Simplifications:
Set-FabricApiHeaders
withConnect-FabricAccount
. (CHANGELOG.md
CHANGELOG.mdR45-R47)Test-TokenExpired
and removing redundant parameters. (source/Private/Test-TokenExpired.ps1
[1] [2]Testing Configuration Adjustments:
CodeCoverageThreshold
inbuild.yaml
from0.35
to0.30
for Pester tests. (build.yaml
build.yamlL107-R107)Fixed
Invoke-FabricAPIRequest
fromRemove-FabricWarehouse
. (Remove-FabricWarehouse fix #80)Removed
Invoke-FabricAPIRequest
and replaced it byInvoke-FabricRestMethodExtended
Confirm-FabricAuthToken
and extended existingTest-TokenExpired
usingEnableTokenRefresh
Feature FlagSet-FabricApiHeaders
and merged the entire logic toConnect-FabricAccount
Task list
build.ps1 -ResolveDependency
).and comment-based help.