Refactored FS scanner to use factory model and work better with mocking #21894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rnpmsft
requested review from
amishra-dev,
seanmcc-msft and
tg-msft
as code owners
ghost
added
the
Comment on lines
20
to
21
| // Constuctor for mocking | ||
| public DirectoryScanner() { } |
There was a problem hiding this comment.
why are we doing this ?
There was a problem hiding this comment.
I saw that in ChangeFeed and its tests, the Segment, SegmentFactory, etc had these kinds of constructors so their outputs could be manipulated for mocking in the tests; I was thinking this could be for mocking specific file structures, so setting up maybe 3 DirectoryScanners, fixing the outputs, and the fixing the DirectoryScannerFactory output to these three scanners. I guess this is kind of unnecessary if we can just do temp files, though.
There was a problem hiding this comment.
In Changefeed I believe these were created for Moq (https://github.com/Moq/moq4/wiki/Quickstart ). But it turns out these are unnecessary. Moq can deal with parametrized ctors.
There was a problem hiding this comment.
Cool, I'll take these out across the refactor (and since I can move the methods in here to private methods in FileSystemScanner anyways if the mocking is needed, this class and its factory will disappear)
Comment on lines
19
to
20
| // Constuctor for mocking | ||
| public DirectoryScannerFactory() { } |
|
|
||
| public DirectoryScanner(string path) | ||
| { | ||
| _path = path; |
There was a problem hiding this comment.
what if this isn't directory?
There was a problem hiding this comment.
I think I assumed everything would go through FileSystemScanner in expected manner, but this is a possible case if mocking or just using DirectoryScanner elsewhere without FileSystemScanner. I'll throw an exception if a path isn't a directory.
| private Queue<string> _directories; | ||
| private Queue<string> _files = new Queue<string>(); | ||
|
|
||
| public FileSystemScanner(string path, bool isDir, DirectoryScannerFactory dirFactory = null) |
There was a problem hiding this comment.
- Do we need
isDir? shouldn'tFileSystemScannerbe able to check that? dirFactoryseems to be essential for this thing to work, why is it optional?
There was a problem hiding this comment.
It could, I had isDir there as the factory would check and then pass that in to set up the scanner properly for if a directory is being checked, but like you said if Scan() is never called then the setup is just unnecessary.
dirFactory is only necessary if we're scanning a directory. If a file is passed in, there's no need to call the "enumerate children" functions; we just return that one file.
I added the DirectoryScanner+Factory in just to cover the possible case of mocking a folder structure without needing to create a temp structure, but if that's not necessary to worry about, this could all pretty much be reverted to the previous setup without DirectoryScanner. The only thing different then would be the added Factory setup to create a scanner.
Comment on lines
29
to
52
| if (isDir) | ||
| { | ||
| try | ||
| { | ||
| // Set up DirectoryScanner and initialize path queues | ||
| _dirFactory = dirFactory; | ||
| _currentDirectory = dirFactory.BuildDirectoryScanner(path); | ||
| _directories = new Queue<string>(); | ||
| Populate(); | ||
| } | ||
| catch | ||
| { | ||
| // If we lack permissions to enumerate the caller-provided folder, | ||
| // throw the error | ||
| // | ||
| // TODO: Logging for missing permissions to enumerate main folder | ||
| throw; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // Add the single file to file queue, will be the only output | ||
| _files.Enqueue(path); | ||
| } |
There was a problem hiding this comment.
I'm not sure we should do heavy work in ctor. What if nobody calls Scan ?
Comment on lines
55
to
56
| // Constuctor for mocking | ||
| public FileSystemScanner() { } |
| FileSystemScannerFactory folderFactory = new FileSystemScannerFactory(folder); | ||
| FileSystemScanner scanFolder = folderFactory.BuildFilesystemScanner(); | ||
| FileSystemScannerFactory fileFactory = new FileSystemScannerFactory(file); | ||
| FileSystemScanner scanFile = fileFactory.BuildFilesystemScanner(); |
There was a problem hiding this comment.
do we have any test that covers sunny day scenario with nested folders ?
There was a problem hiding this comment.
The first test sets up a folder with two direct children files (one is unreadable), and two subfolders, one with and one without read permissions. The tests succeeds with expected output being the two direct children and the child inside the readable subfolder, so that covers both nested folder cases. Should this be split up from one test?
| // Constuctor for mocking | ||
| public DirectoryScanner() { } | ||
|
|
||
| public IEnumerable<string> EnumerateDirectories() |
There was a problem hiding this comment.
Why do we call this EnumerateX while we call it Scan elsewhere?
Would it be better to return some model from here which has both name and type (file vs directory) and collapse these methods?
There was a problem hiding this comment.
This was just making an abstraction of the built in methods so these could be mocked. Now that I'm thinking about it, these helper abstractions don't necessitate a whole new class; I can move these into private methods inside the FileSystemScanner.
|
|
||
| namespace Azure.Storage.Common.DataMovement | ||
| { | ||
| internal class FileSystemScanner |
There was a problem hiding this comment.
I'm not sure this should be called FileSystemScanner if we're pointing it to a path rather than root.
Should this be PathScanner ?
Or maybe we should have IPathScanner and implementations like DirectoryScanner and FileScanner ? But then what does it mean to scan a file?
There was a problem hiding this comment.
Fair point. Probably a PathScanner would be best, so that a caller only has to worry about coming up with the path, and the scanner does the work of figuring out whether its just one or several items. The separate implementations would make little sense (at least with the latter; FileScanner logic would pretty much amount to File.GetAttributes(fullPath) & FileAttributes.Directory) != FileAttributes.Directory not throwing, which probably doesn't need a whole class)
There was a problem hiding this comment.
Maybe we're doing this split onto dir/file scanners prematurely. Should we just have PathScanner that encapsulates all this logic for now and see how it evolves?
@amnguye @gapra-msft thoughts ?
There was a problem hiding this comment.
I was actually just now thinking the same thing. I think the FilesystemScanner -> PathScanner should be able to handle both files and directories. No need for a Directory/FileScanner for now
amnguye
reviewed
Jun 17, 2021
sdk/storage/Azure.Storage.Common.DataMovement/src/DirectoryScannerFactory.cs
Outdated
Show resolved
Hide resolved
amnguye
reviewed
Jun 17, 2021
sdk/storage/Azure.Storage.Common.DataMovement/src/DirectoryScanner.cs
Outdated
Show resolved
Hide resolved
amnguye
approved these changes
Jul 6, 2021
kasobol-msft
approved these changes
Jul 6, 2021
amnguye
pushed a commit
that referenced
this pull request
Dec 6, 2021
…ng (#21894) * Refactored FS scanner to use factory model and work better with mocking * Rename class/simplify factory implementation * Return folders as well; preview of ctor logic changes (only throw if path nonexistent/malformed) * Changed parameter name for scan (continueOnError), re-exported API * More exported API changes
amnguye
added a commit
that referenced
this pull request
Dec 5, 2022
* Create packages for DM Common and Blobs * Making Test packages for DM Common and Blobs; Added Readme, Changelog, BreakingChanges stubs * WIP - Added BlobDirectoryUploadOptions and StorageTransferStatus * Initial creation of filesystem scanner for DMLib (#21492) * Filesystem scanner refactored to non-static implementation (#21715) * Created filesystem scanner for DM Common * Modifed scanner to properly handle missing permissions; added test cases for filesystem scanner * Tests remade using native temp files; Scanner now throws errors that precede first successful yield * Changed Posix compatibility dependency * Edited versioning and READMEs to adhere to pipelines * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Scanner will only work on one path for now * Capitalization on FileSystemScanner * Changed scanner to internal * Refactored FS scanner to use factory model and work better with mocking (#21894) * Refactored FS scanner to use factory model and work better with mocking * Rename class/simplify factory implementation * Return folders as well; preview of ctor logic changes (only throw if path nonexistent/malformed) * Changed parameter name for scan (continueOnError), re-exported API * More exported API changes * DMLib Skeleton start (#22336) * WIP - Removed DataMovement Blobs package, consildate to one package * WIP - Storage Transfer Jobs * WIP - remove dm blobs * WIP - Added TraansferItemScheduler * Ran exportapis * WIP - Resolve package conflicts * Addressed most PR comments * Ran export-api script * Made job for each specific operation for blobs * Added specific copy directory jobs, added option bags for copy scenarios * Ran ExportApi script * Update comments in StorageTransferManager * Rename BlobUploadDirectoryOptions -> BlobDirectoryUploadOptions * Run ExportAPI * PR Comments * Merge fix * WIP * Directory Upload and Download basic tests work * Test recordings test * Rerecord tests * WIP - not all ListBlobs/GetBlobs tests for DirectoryClient pass * WIP - blobtransfermanager * WIP - Moving configuations for DM Blobs * WIP - blobtransferjobs * Updated storage solution file * WIP - pathScanner tests * WIP - champion scenarios * WIP - champ scenarios * WIP - small changes * WIP' * WIP * WIP * Create packages for DM Common and Blobs * Making Test packages for DM Common and Blobs; Added Readme, Changelog, BreakingChanges stubs * WIP - Added BlobDirectoryUploadOptions and StorageTransferStatus * Initial creation of filesystem scanner for DMLib (#21492) * Filesystem scanner refactored to non-static implementation (#21715) * Created filesystem scanner for DM Common * Modifed scanner to properly handle missing permissions; added test cases for filesystem scanner * Tests remade using native temp files; Scanner now throws errors that precede first successful yield * Changed Posix compatibility dependency * Edited versioning and READMEs to adhere to pipelines * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Scanner will only work on one path for now * Capitalization on FileSystemScanner * Changed scanner to internal * Refactored FS scanner to use factory model and work better with mocking (#21894) * Refactored FS scanner to use factory model and work better with mocking * Rename class/simplify factory implementation * Return folders as well; preview of ctor logic changes (only throw if path nonexistent/malformed) * Changed parameter name for scan (continueOnError), re-exported API * More exported API changes * DMLib Skeleton start (#22336) * WIP - Removed DataMovement Blobs package, consildate to one package * WIP - Storage Transfer Jobs * WIP - remove dm blobs * WIP - Added TraansferItemScheduler * Ran exportapis * WIP - Resolve package conflicts * Addressed most PR comments * Ran export-api script * Made job for each specific operation for blobs * Added specific copy directory jobs, added option bags for copy scenarios * Ran ExportApi script * Update comments in StorageTransferManager * Rename BlobUploadDirectoryOptions -> BlobDirectoryUploadOptions * Run ExportAPI * PR Comments * Merge fix * Merge main update * WIP * Builds here without Azure.Storage.DataMovement.Blobs * Builds - DMLib common, DMlib blobs, DMlib samples * Added back in blobs tests * BlobTransferScheduler updated, logger updated, plan file updated * API generates * Rerun some tests, attempting to fix some parallel start problems * Resolve bad merge conflicts * DMLib builds but Blobs.Tests does not build * Conversion from internal job to job details * Run exports api update * Update logger information * Changed threadpool method to use inherit TransferScheduler * Remove previous implementation of getting job details, and combine into one * Removing mistake of committed files * Update to Job Plan header * Updating manager detail of API * Add abstract resumeJob to base storagetransfermanager * Update event arguments to have individual ones for each case, update progress handler to basic handler, update copy method * Removed base DM models, made base event args, made protected ctor StorageTransferManager * Changed Directory Download to DownloadTo, added overwrite options, updated internal pipeline transfer for directoryclient * change string to uri for local paths, remove unncessary things from blob job properties * WIP - changing job details out, added more champ scenarios regarding progress tracking * Updating Resume API, correcting event arg names, correctly linked internal deep copy for directory client * Readded upload directory tests with working json files, changed uploadDirectory API return type, Mild changes to some APIs, renamed part files * WIP * Cannot catch exception properly, tear downs upload call * Addressing Arch board comment * Some fixes from merging from main Remove test dependency on AesGcm for datamovement * WIP * Renamed Experimental to DataMovement * Fixed channel blocklist issue * WIP - changing event handler in uploader to trigger block status * Working commit block handler * WIP * Changes to Download and APIs regarding download * Copy Service Job cleanup * WIP - API changes to StorageResource and Controller * WIP * WIP - Aligning blobs API usage * WIP - Added dependenices to Azure.Storage.DataMovement.Test * WIP - Updated APIs to include checkpointing * WIP - ConsumeableStream -> GetConsumerableStream * WIP - make old API structure internal; todo: remove all old APIs * WIP - Remade API for blobs DM, removed CopyMethod * WIP -Update to StorageTransfer event args name * WIP - Removed factory calls, made dervived storage resource types public * Merged BlobDataController to main controller, renamed DataController to Transfermanager, removed ListType from StorageResource * WIP - Added Checkpointer API, removed unnecessary -1 enum values, updated job plan header bytes * WIP - removed options from respective derived storage resource calls, added options bag to blob storage resources * WIP - renamed CommitListTYpe to clearer type * WIP - Update to Copy Options api in blockblob storage, and samples * WIP - Updated APIs * WIP - Updated APIs to include offset streams * WIP - Rename writetooffsetoptions with storageresource prefixed * WIP - copy to and from and update to mmp job plan file * Added over the concurrency tuner * Remove ConfigureAwait from samples * WIP - changes to MMF, service to service copy and adding method to pass the token credential or bearer token to storage resource * WIP - fixes to event handler, removable of complete transfer check api * WIP - fix to closing stream when reading from local, setting blocklist order before commiting * WIP - tests * WIP - Remove unnecessary APIs and old code * Removing more unnecessary changes and test recordings for old tests * More removal of old test recordings * Removing BlobFolderClient / BlobVirtualDirectoryClient * Ran Export APIs, moved DataTransferExtensions to DataTransfer * ApiView Comments addressed * Renamed from Blobs.DataMovement to DataMovement.Blobs * Ran ExportApis * Updating assemblyinfo datamovement blobs namespace * Move over Storage Resource tests; Made some API corrections * Remove suppression on editorconfig * Added API for creation of blobs storage resource, max chunk size, more tests, fixes * Changed GetStorageResources to return a base class of storage resource; fixed bugs with append / sequential operations; Updated copy status handler for async copy * PR Comments - reverted necessary config files, moved constants to a separate file, rremvoed globalsupression files * Export APIs * PR Comments - removed merge mistakes, updated some xml comments, change some option bags, removed blobstorageresourcefactory, removed more globalsupression files * PR Comments - Move unnecessary return xml removed and removed localfilefactory * PR Comments - Removing leftover folder models from BlobVirtualFolderClient * Updating GetProperties comment XML, removing first value from cpu monitor reading, adding try block to delete file when failed download chunks occur * Fix to directory, and some test changes to use DataTransfer awaitcompletion * Update to tests and adding discovered length optimization * Ignore some tests for now, to push recording in a separate PR * Update readmes * Ignore more tests * Ignore more local directory tests * Temporarily remove nuget package link; readd when link works when package is released * Update snippets to include length Co-authored-by: Rushi Patel <[email protected]>
sofiar-msft
pushed a commit
to sofiar-msft/azure-sdk-for-net
that referenced
this pull request
Dec 7, 2022
* Create packages for DM Common and Blobs * Making Test packages for DM Common and Blobs; Added Readme, Changelog, BreakingChanges stubs * WIP - Added BlobDirectoryUploadOptions and StorageTransferStatus * Initial creation of filesystem scanner for DMLib (Azure#21492) * Filesystem scanner refactored to non-static implementation (Azure#21715) * Created filesystem scanner for DM Common * Modifed scanner to properly handle missing permissions; added test cases for filesystem scanner * Tests remade using native temp files; Scanner now throws errors that precede first successful yield * Changed Posix compatibility dependency * Edited versioning and READMEs to adhere to pipelines * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Scanner will only work on one path for now * Capitalization on FileSystemScanner * Changed scanner to internal * Refactored FS scanner to use factory model and work better with mocking (Azure#21894) * Refactored FS scanner to use factory model and work better with mocking * Rename class/simplify factory implementation * Return folders as well; preview of ctor logic changes (only throw if path nonexistent/malformed) * Changed parameter name for scan (continueOnError), re-exported API * More exported API changes * DMLib Skeleton start (Azure#22336) * WIP - Removed DataMovement Blobs package, consildate to one package * WIP - Storage Transfer Jobs * WIP - remove dm blobs * WIP - Added TraansferItemScheduler * Ran exportapis * WIP - Resolve package conflicts * Addressed most PR comments * Ran export-api script * Made job for each specific operation for blobs * Added specific copy directory jobs, added option bags for copy scenarios * Ran ExportApi script * Update comments in StorageTransferManager * Rename BlobUploadDirectoryOptions -> BlobDirectoryUploadOptions * Run ExportAPI * PR Comments * Merge fix * WIP * Directory Upload and Download basic tests work * Test recordings test * Rerecord tests * WIP - not all ListBlobs/GetBlobs tests for DirectoryClient pass * WIP - blobtransfermanager * WIP - Moving configuations for DM Blobs * WIP - blobtransferjobs * Updated storage solution file * WIP - pathScanner tests * WIP - champion scenarios * WIP - champ scenarios * WIP - small changes * WIP' * WIP * WIP * Create packages for DM Common and Blobs * Making Test packages for DM Common and Blobs; Added Readme, Changelog, BreakingChanges stubs * WIP - Added BlobDirectoryUploadOptions and StorageTransferStatus * Initial creation of filesystem scanner for DMLib (Azure#21492) * Filesystem scanner refactored to non-static implementation (Azure#21715) * Created filesystem scanner for DM Common * Modifed scanner to properly handle missing permissions; added test cases for filesystem scanner * Tests remade using native temp files; Scanner now throws errors that precede first successful yield * Changed Posix compatibility dependency * Edited versioning and READMEs to adhere to pipelines * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Refactored scanner to non-static implementation; provided user configurable options for error handling * Removed test dependencies from central package list * Scanner will only work on one path for now * Capitalization on FileSystemScanner * Changed scanner to internal * Refactored FS scanner to use factory model and work better with mocking (Azure#21894) * Refactored FS scanner to use factory model and work better with mocking * Rename class/simplify factory implementation * Return folders as well; preview of ctor logic changes (only throw if path nonexistent/malformed) * Changed parameter name for scan (continueOnError), re-exported API * More exported API changes * DMLib Skeleton start (Azure#22336) * WIP - Removed DataMovement Blobs package, consildate to one package * WIP - Storage Transfer Jobs * WIP - remove dm blobs * WIP - Added TraansferItemScheduler * Ran exportapis * WIP - Resolve package conflicts * Addressed most PR comments * Ran export-api script * Made job for each specific operation for blobs * Added specific copy directory jobs, added option bags for copy scenarios * Ran ExportApi script * Update comments in StorageTransferManager * Rename BlobUploadDirectoryOptions -> BlobDirectoryUploadOptions * Run ExportAPI * PR Comments * Merge fix * Merge main update * WIP * Builds here without Azure.Storage.DataMovement.Blobs * Builds - DMLib common, DMlib blobs, DMlib samples * Added back in blobs tests * BlobTransferScheduler updated, logger updated, plan file updated * API generates * Rerun some tests, attempting to fix some parallel start problems * Resolve bad merge conflicts * DMLib builds but Blobs.Tests does not build * Conversion from internal job to job details * Run exports api update * Update logger information * Changed threadpool method to use inherit TransferScheduler * Remove previous implementation of getting job details, and combine into one * Removing mistake of committed files * Update to Job Plan header * Updating manager detail of API * Add abstract resumeJob to base storagetransfermanager * Update event arguments to have individual ones for each case, update progress handler to basic handler, update copy method * Removed base DM models, made base event args, made protected ctor StorageTransferManager * Changed Directory Download to DownloadTo, added overwrite options, updated internal pipeline transfer for directoryclient * change string to uri for local paths, remove unncessary things from blob job properties * WIP - changing job details out, added more champ scenarios regarding progress tracking * Updating Resume API, correcting event arg names, correctly linked internal deep copy for directory client * Readded upload directory tests with working json files, changed uploadDirectory API return type, Mild changes to some APIs, renamed part files * WIP * Cannot catch exception properly, tear downs upload call * Addressing Arch board comment * Some fixes from merging from main Remove test dependency on AesGcm for datamovement * WIP * Renamed Experimental to DataMovement * Fixed channel blocklist issue * WIP - changing event handler in uploader to trigger block status * Working commit block handler * WIP * Changes to Download and APIs regarding download * Copy Service Job cleanup * WIP - API changes to StorageResource and Controller * WIP * WIP - Aligning blobs API usage * WIP - Added dependenices to Azure.Storage.DataMovement.Test * WIP - Updated APIs to include checkpointing * WIP - ConsumeableStream -> GetConsumerableStream * WIP - make old API structure internal; todo: remove all old APIs * WIP - Remade API for blobs DM, removed CopyMethod * WIP -Update to StorageTransfer event args name * WIP - Removed factory calls, made dervived storage resource types public * Merged BlobDataController to main controller, renamed DataController to Transfermanager, removed ListType from StorageResource * WIP - Added Checkpointer API, removed unnecessary -1 enum values, updated job plan header bytes * WIP - removed options from respective derived storage resource calls, added options bag to blob storage resources * WIP - renamed CommitListTYpe to clearer type * WIP - Update to Copy Options api in blockblob storage, and samples * WIP - Updated APIs * WIP - Updated APIs to include offset streams * WIP - Rename writetooffsetoptions with storageresource prefixed * WIP - copy to and from and update to mmp job plan file * Added over the concurrency tuner * Remove ConfigureAwait from samples * WIP - changes to MMF, service to service copy and adding method to pass the token credential or bearer token to storage resource * WIP - fixes to event handler, removable of complete transfer check api * WIP - fix to closing stream when reading from local, setting blocklist order before commiting * WIP - tests * WIP - Remove unnecessary APIs and old code * Removing more unnecessary changes and test recordings for old tests * More removal of old test recordings * Removing BlobFolderClient / BlobVirtualDirectoryClient * Ran Export APIs, moved DataTransferExtensions to DataTransfer * ApiView Comments addressed * Renamed from Blobs.DataMovement to DataMovement.Blobs * Ran ExportApis * Updating assemblyinfo datamovement blobs namespace * Move over Storage Resource tests; Made some API corrections * Remove suppression on editorconfig * Added API for creation of blobs storage resource, max chunk size, more tests, fixes * Changed GetStorageResources to return a base class of storage resource; fixed bugs with append / sequential operations; Updated copy status handler for async copy * PR Comments - reverted necessary config files, moved constants to a separate file, rremvoed globalsupression files * Export APIs * PR Comments - removed merge mistakes, updated some xml comments, change some option bags, removed blobstorageresourcefactory, removed more globalsupression files * PR Comments - Move unnecessary return xml removed and removed localfilefactory * PR Comments - Removing leftover folder models from BlobVirtualFolderClient * Updating GetProperties comment XML, removing first value from cpu monitor reading, adding try block to delete file when failed download chunks occur * Fix to directory, and some test changes to use DataTransfer awaitcompletion * Update to tests and adding discovered length optimization * Ignore some tests for now, to push recording in a separate PR * Update readmes * Ignore more tests * Ignore more local directory tests * Temporarily remove nuget package link; readd when link works when package is released * Update snippets to include length Co-authored-by: Rushi Patel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All SDK Contribution checklist:
This checklist is used to make sure that common guidelines for a pull request are followed.
Draftmode if it is:General Guidelines and Best Practices
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK. Please double check nuget.org current release version.Additional management plane SDK specific contribution checklist:
Note: Only applies to
Microsoft.Azure.Management.[RP]orAzure.ResourceManager.[RP]Management plane SDK Troubleshooting
If this is very first SDK for a services and you are adding new service folders directly under /SDK, please add
new servicelabel and/or contact assigned reviewer.If the check fails at the
Verify Code Generationstep, please ensure:generate.ps1/cmdto generate this PR instead of callingautorestdirectly.Please pay attention to the @microsoft.csharp version output after running
generate.ps1. If it is lower than current released version (2.3.82), please run it again as it should pull down the latest version.Note: We have recently updated the PSH module called by
generate.ps1to emit additional data. This would help reduce/eliminate the Code Verification check error. Please run following command:Old outstanding PR cleanup
Please note:
If PRs (including draft) has been out for more than 60 days and there are no responses from our query or followups, they will be closed to maintain a concise list for our reviewers.