Skip to content

Conversation

@blueww
Copy link
Member

@blueww blueww commented Aug 28, 2018

Description

Oauth design pass review in: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/37 , milestone 1
StaticWeb design pass review in Azure/azure-powershell-cmdlet-review-pr#88

Since the 2 features both need XSCL upgrade to 9.3, and most of the changes are on XSCL upgrade, so raise them in same PR. (actually the code change al ready pass review and merge to storage branch in #5960 and #6518)

Checklist

[Wei] The breaking change is remove.

  • If applicable, the changes made in the PR have proper test coverage

[Wei] Will add cases after #7037 is in.

@blueww
Copy link
Member Author

blueww commented Aug 29, 2018

@markcowl@MiYanni
The netcore build fail with following error, I even tried to upgrade the XSCL to 9.3 to align with storage project, but still fail. Any idea?

[xUnit.net 00:00:07.89] Microsoft.Azure.Commands.Compute.Test.ScenarioTests.ComputeCloudExceptionTests.RunComputeCloudExceptionTests [FAIL]

Failed Microsoft.Azure.Commands.Compute.Test.ScenarioTests.ComputeCloudExceptionTests.RunComputeCloudExceptionTests

Error Message:

System.Management.Automation.RuntimeException : Expected exception is not like the expected text 'Resourcenot found*OperationID : *', the actual message is 'The term 'Get-AzVM' is not recognized as the name of a cmdlet, function, script file, or operable program.

@maddieclayton
Copy link
Contributor

@blueww Looks like it is related to this error:
/usr/share/dotnet/sdk/2.1.400/Microsoft.Common.CurrentVersion.targets(2110,5): warning MSB3277: Found conflicts between different versions of "Microsoft.WindowsAzure.Storage" that could not be resolved. These reference conflicts are listed in the build log when log verbosity is set to detailed. [/home/travis/build/Azure/azure-powershell/src/ResourceManager/Compute/Commands.Compute.Test/Commands.Compute.Test.Netcore.csproj]

@blueww
Copy link
Member Author

blueww commented Aug 30, 2018

@maddieclayton

I have upgrade both Commands.Storage.Test.Netcore.csproj and Commands.Storage.Netcore.csproj to XSCL 9.3.0, but the error still happen.

in .net, we only need to upgrade XSCL for storage project; but in netcore, seems we need to upgrade to many projects. Could we make it also only need upgrade XSCL for storage project?
And for this PR, could you please point out which projects we need to upgrade XSCL when upgrade XSCL for storage?

Thanks!

@MiYanni
Copy link
Contributor

MiYanni commented Aug 30, 2018

@blueww In NetCore, every module has to rely on the same version of any dependency. The Storage dependency used in one place needs to be updated everywhere it is used.

@blueww
Copy link
Member Author

blueww commented Aug 31, 2018

@MiYanni , @maddieclayton
I have searched for all netcore projects which reference "WindowsAzure.Storage" 9.0, and make them all reference 9.3, but the netcore build (test) still fail with following error, any idea?

PowerShell Error Record: Could not load file or assembly 'Microsoft.WindowsAzure.Storage, Version=9.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.

Exception:System.IO.FileLoadException: Could not load file or assembly 'Microsoft.WindowsAzure.Storage, Version=9.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.

at System.Management.Automation.Runspaces.InitialSessionState.Bind_LoadAssemblies(ExecutionContext context)

at System.Management.Automation.Runspaces.InitialSessionState.Bind(ExecutionContext context, Boolean updateOnly, PSModuleInfo module, Boolean noClobber, Boolean local)

at System.Management.Automation.Runspaces.InitialSessionState.Bind(ExecutionContext context, Boolean updateOnly)

at Microsoft.PowerShell.Commands.ModuleCmdletBase.LoadModuleManifest(String moduleManifestPath, ExternalScriptInfo scriptInfo, Hashtable data, Hashtable localizedData, ManifestProcessingFlags manifestProcessingFlags, Version minimumVersion, Version maximumVersion, Version requiredVersion, Nullable`1 requiredModuleGuid, ImportModuleOptions& options, Boolean& containedErrors)

The projects has been upgraded to XSCL 9.3 are following, any missing

?
C:\code\PSH_Dev\src\Common\Commands.Common.Storage\Common.Storage.Netcore.csproj(29):
C:\code\PSH_Dev\src\Common\Commands.Common.Storage.Management\Common.Storage.Management.Netcore.csproj(25):
C:\code\PSH_Dev\src\ResourceManager\ApiManagement\Commands.ApiManagement\Commands.ApiManagement.Netcore.csproj(33):
C:\code\PSH_Dev\src\ResourceManager\AzureBatch\Commands.Batch\Commands.Batch.Netcore.csproj(34):
C:\code\PSH_Dev\src\ResourceManager\AzureBatch\Commands.Batch.Test\Commands.Batch.Test.Netcore.csproj(30):
C:\code\PSH_Dev\src\ResourceManager\Compute\Commands.Compute\Commands.Compute.Netcore.csproj(37):
C:\code\PSH_Dev\src\ResourceManager\RedisCache\Commands.RedisCache\Commands.RedisCache.Netcore.csproj.orig(35):
C:\code\PSH_Dev\src\ResourceManager\Storage\Commands.Management.Storage\Commands.Management.Storage.Netcore.csproj(37):
C:\code\PSH_Dev\src\Storage\Commands.Storage\Commands.Storage.Netcore.csproj(32):
C:\code\PSH_Dev\src\Storage\Commands.Storage.Test\Commands.Storage.Test.Netcore.csproj(27):

@MiYanni
Copy link
Contributor

MiYanni commented Aug 31, 2018

@blueww Please do not change any Common files in this repo as they are not used. The equivalent files for the ones you changed are here in our azure-powershell-common repo:
https://github.com/Azure/azure-powershell-common/blob/develop/src/Storage/Storage.Netcore.csproj
https://github.com/Azure/azure-powershell-common/blob/develop/src/Storage.Management/Storage.Management.Netcore.csproj

I will be removing Common code from this codebase within the next couple business days. I also plan on consolidating references to the Common packages in this codebase. If you make the changes to those files in azure-powershell-common, and create a build here, https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-common-sign/, you can drop the packages into the tools/LocalFeed folder, and replace the references to 1.0.94-preview in this codebase with whichever version you get from the packages. Then, you should be good.

@blueww
Copy link
Member Author

blueww commented Aug 31, 2018

@MiYanni
I am not sure why the following files are common file, they seems should be storage project file, and should not be common.

https://github.com/Azure/azure-powershell-common/blob/develop/src/Storage/Storage.Netcore.csproj
https://github.com/Azure/azure-powershell-common/blob/develop/src/Storage.Management/Storage.Management.Netcore.csproj

And it seems I don't have direct write access to them. should I raise a PR for it?
Would you please point out all files that I should change in azure-powershell-common, and I will send a PR for it together?

And would you please point out all Packages that I should get from https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-common-sign/, for XSCL , or "Microsoft.Azure.Management.Storage" upgrade.
And is there any way that I can test it before the PR to azure-powershell-common is merged.

And is there any formal doc for this process? I still have many questions. I will schedule a meeting to discuss it with you.

@MiYanni
Copy link
Contributor

MiYanni commented Aug 31, 2018

@blueww These are common SDK dlls for the Azure PowerShell runtime. The only difference is we moved the projects to a separate repo and are going to make them provided by a MyGet feed for this repo. Storage and Storage.Management are dataplane and management DLLs respectively of 'locked' versions of those SDKs that are shared by the modules within this repo. Otherwise, if someone wanted to update an SDK and several modules referenced that SDK, every time the SDK version was changed, it would require them to re-record all their tests. This way, we lock a shared version of the SDK so that other modules can still use the functionality there without worrying about SDK updates. This has been how anything in the Common folder in this repo has worked for a long time. Again, that folder will be removed very soon.

For the azure-powershell-common repo, simply use https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-common-sign/ to make the packages, and follow the steps I mentioned above. You don't need a PR, just a fork to that repo for now. Prior to this PR being accepted, those changes would need to be accepted into azure-powershell-common. There is no documentation yet as this process is in-flux, as we are transitioning to making Az (NetCore/NetStandard) our primary focus. All documentation will be updated as we transition to Az.

Additionally, the intent of azure-powershell-common is for us to use to manage the Azure PowerShell runtime of this repo. Services should not need to modify this repo often (if ever). The only reason you need to now is that WindowsAzure.Storage is referenced there. We have an open issue to remove WindowsAzure.Storage usage entirely from (almost) everything for Az. So many modules show not rely on it, or this situation happens. Only Storage modules should use this dll. It is an issue from when we were converting modules that we haven't resolved yet. Sorry for the inconvenience.

@blueww
Copy link
Member Author

blueww commented Aug 31, 2018

Thanks @MiYanni for the reply!

Just to confime:

  1. For XSCL upgrade, I should
  1. For SRP SDK ("Microsoft.Azure.Management.Storage") upgrade, should:

@MiYanni
Copy link
Contributor

MiYanni commented Aug 31, 2018

@blueww 1. The current version number of the common packages in this repo, I believe, is 1.0.94. Right now, from the build from your fork, simply replace all common packages in LocalFeed. Then, for all of azure-powershell, just do a find/replace for 1.0.94 with your build number (it is created automatically from the sign build). We will have individual package replacement, but we don't do that yet (I'm working on it).
2. I don't believe any Netcore builds in azure-powershell-common use Microsoft.Azure.Management.Storage. Double check that as I might be wrong.

@blueww
Copy link
Member Author

blueww commented Sep 4, 2018

@MiYanni

I have tried to change all azure-powershell-common packages to 1.0.96, but build still fail.
I think we still need a meeting to go through the change process.
I have schedule a meeting on Tuesday 6:00pm Redmond time, propose a new time if the time is not proper for you.

# Conflicts:
#	src/Common/Commands.Common.Storage.Management/Common.Storage.Management.Netcore.csproj
#	src/Common/Commands.Common.Storage/Common.Storage.Netcore.csproj
@MiYanni
Copy link
Contributor

MiYanni commented Sep 5, 2018

@azuresdkci Retest this please

@blueww blueww changed the title [Storage] Support Oauth in Powershell official release [Storage] Support Oauth/StaticWeb in Powershell official release Sep 5, 2018
@blueww
Copy link
Member Author

blueww commented Sep 5, 2018

@MiYanni
Thanks for fixing this!
I have add another feture to this PR (static Web), since the feature also need upgrade XSCL to 9.3. This feature also pass review already in the storage branch.
On Demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/1082/
ps-sign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/1159/
psnetsign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign/153/

Please help to review and merge it ASAP. Let me know if any thing need change.

/// <summary>
/// Disable azure storage service StaticWebsite, currently only available on Blob service
/// </summary>
[Cmdlet(VerbsLifecycle.Disable, StorageNouns.ServiceStaticWebsite, SupportsShouldProcess = true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be named Azure.Commands.ResourceManager.Common.AzureRMConstants.AzurePrefix + "StorageStaticWebsite"

/// <summary>
/// Enable azure storage service StaticWebsite, currently only enabled on Blob service
/// </summary>
[Cmdlet(VerbsLifecycle.Enable, StorageNouns.ServiceStaticWebsite, SupportsShouldProcess = true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be named Azure.Commands.ResourceManager.Common.AzureRMConstants.AzurePrefix + "StorageStaticWebsite"


/// Azure storage Service Delete Retention Policy
/// </summary>
public const string ServiceStaticWebsite = "AzureStorageStaticWebsite";
Copy link
Contributor

Choose a reason for hiding this comment

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

These constants are not used for cmdlet names. No need to add a new constant.

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.

4 participants