Skip to content

Conversation

@blueww
Copy link
Member

@blueww blueww commented Jul 30, 2020

Description

design pass review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/676
This PR also upgrade Azure.Core to 1.3.0, since Storage SDK dependency upgrade.
Please merge it after 8/4 PSH release is out. (the change log might need resolve conflict.)

By upgrade SDK, also can resolve the issue: #12553

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@blueww blueww requested a review from erich-wang July 30, 2020 07:04
@isra-fel isra-fel self-assigned this Jul 30, 2020
@blueww
Copy link
Member Author

blueww commented Jul 31, 2020

@erich-wang
Would you please suggest how to fix the Az.Synapse test case fail, related with Azure.Core 1.3.0:

PowerShell Error Record : Test failed due to a non-empty error stream. First error : PowerShell Error Record: Could not load file or assembly 'Azure.Core, Version=1.3.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'. Could not find or load a specific file. (Exception from HRESULT: 0x80131621) [D:\a\1\s\build.proj]
Exception:System.IO.FileLoadException: Could not load file or assembly 'Azure.Core, Version=1.3.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'. Could not find or load a specific file. (Exception from HRESULT: 0x80131621)
File name: 'Azure.Core, Version=1.3.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8' ---> System.IO.FileLoadException: Could not load file or assembly 'Azure.Core, Version=1.3.0.0, Culture=neutral, PublicKeyToken=92742159e12e44c8'.

@blueww
Copy link
Member Author

blueww commented Jul 31, 2020

Add 2 static analysis exception:

  1. New-AzStorageBlobQueryConfig don't need Should process, since it will only create a local object, won't change anything on server.
  2. Query-AzStorageBlob, "Query" is not an approved verb.
  • The current cmdlet name pass design review in : Azure/azure-powershell-cmdlet-review-pr#676
  • I can't find an approved verb which match the function of: Query a blob content with a query expression, and only return the subset of blob content which matches the query expression.
  • If it's required to change the verb, please suggest the verb, and cmdlet name?

@isra-fel
Copy link
Member

isra-fel commented Aug 3, 2020

Add 2 static analysis exception:

  1. New-AzStorageBlobQueryConfig don't need Should process, since it will only create a local object, won't change anything on server.
  2. Query-AzStorageBlob, "Query" is not an approved verb.
  • The current cmdlet name pass design review in : Azure/azure-powershell-cmdlet-review-pr#676
  • I can't find an approved verb which match the function of: Query a blob content with a query expression, and only return the subset of blob content which matches the query expression.
  • If it's required to change the verb, please suggest the verb, and cmdlet name?

The first suppression is valid, but I'm afraid we can't accept the second. Let's discuss in the design issue Azure/azure-powershell-cmdlet-review-pr#676

@blueww
Copy link
Member Author

blueww commented Aug 6, 2020

@isra-fel
I have removed the 2nd breaking change exception, and change the cmdlet name from "Query-AzStorageBlob" to "Get-AzStorageBlobQueryResult".
Please help to review again.

Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@blueww
Copy link
Member Author

blueww commented Aug 7, 2020

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@isra-fel isra-fel merged commit 526a74b into Azure:master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants