[ADLA - ADLS] - Object refactoring #4718

ro-joowan · 2017-10-05T03:07:46Z

Based on this Swagger PR: Refactoring changes in order to match with Ben's refactoring changes azure-rest-api-specs#1452
Based on this .NET PR: BREAKING CHANGE: Adl object refactor azure-sdk-for-net#3512
Updated the tests ever so slightly to make them more reliable. I also ran them live and updated the SessionRecords folder for both ADLA and ADLS
Besides code refactoring for type changes, the only real change is updating adapting to how Job submission/build works in src\ResourceManager\DataLakeAnalytics\Commands.DataLakeAnalytics\Commands\SubmitAzureRmDataLakeAnalyticsJob.cs
For the deleted PS(insert some properties object).cs files, it is because via Swagger these properties have been flattened. E.g. in src\ResourceManager\DataLakeAnalytics\Commands.DataLakeAnalytics\Models\PSDataLakeStoreAccountInfo.cs:

[Obsolete("In a future release this object will have all 'Properties' properties flattened and the 'Properties' property will be removed. Until then, nested properies will be duplicated.")]

Updated the changelog for ADLA and ADLS

Description

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

cormacpayne · 2017-10-06T16:55:59Z

@ro-joowan it looks like there are some previously documented breaking changes being made in this release (e.g., removing the Properties property). These will need to be made for the November release rather than the October release. If anything needs to be made in the October release, I would recommend opening a separate PR with those changes so they can get in.

[ADLA] * Changed one of the two OutputTypes of Get-AzureRmDataLakeAnalyticsAccount - List<DataLakeAnalyticsAccount> to List<PSDataLakeAnalyticsAccountBasic> - The properties of PSDataLakeAnalyticsAccountBasic is a strict subset of the properties of DataLakeAnalyticsAccount - The additional properties that are in DataLakeAnalyticsAccount are not returned by the service. Therefore, this change is to reflect this accurately. * Changed one of the two OutputTypes of Get-AzureRmDataLakeAnalyticsJob - List<JobInformation> to List<PSJobInformationBasic> - The properties of PSJobInformationBasic is a strict subset of the properties of JobInformation - The additional properties that are in JobInformation are not returned by the service. Therefore, this change is to reflect this accurately. * Updated the cmdlet logic of Submit-AzureRmDataLakeAnalyticsJob because SubmitJob and BuildJob now require an explicit parameter object rather than the generic JobInformation object * Removed the Obsolete Properties fields in PSDataLakeAnalyticsAccount.cs and its assoicated files [ADLS] * Changed one of the two OutputTypes of Get-AzureRmDataLakeStoreAccount - List<PSDataLakeStoreAccount> to List<PSDataLakeStoreAccountBasic> - The properties of PSDataLakeStoreAccountBasic is a strict subset of the properties of PSDataLakeStoreAccount - The additional properties that are in PSDataLakeStoreAccount are not returned by the service. Therefore, this change is to reflect this accurately. * Removed the Obsolete Properties fields in PSDataLakeStoreAccount.cs and its assoicated files

ro-joowan · 2017-10-09T18:53:07Z

@cormacpayne, This can go in the November release. Thank you!

ro-joowan · 2017-10-15T23:07:56Z

@cormacpayne, Can we move forward from the "Do Not Merge" state?

cormacpayne · 2017-10-18T00:28:39Z

@azuresdkci test this please

cormacpayne

@ro-joowan some initial comments about removing redundant references

cormacpayne · 2017-10-18T00:33:08Z

...sourceManager/DataLakeAnalytics/Commands.DataLakeAnalytics/Commands.DataLakeAnalytics.csproj

+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.DataLake.Analytics.3.1.2-preview\lib\net452\Microsoft.Azure.Management.DataLake.Analytics.dll</HintPath>
+      <Private>True</Private>
+    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">


@ro-joowan please remove the references you are adding to Microsoft.Rest.ClientRuntime, Microsoft.Rest.ClientRuntime.Azure and Newtonsoft.Json in this file. These references are already covered in the Common.Dependencies.targets file in the tools folder.

Done - thanks!

cormacpayne · 2017-10-18T00:38:43Z

src/ResourceManager/DataLakeAnalytics/Commands.DataLakeAnalytics/packages.config

@@ -1,4 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
-  <package id="Microsoft.Azure.Management.DataLake.Analytics" version="3.0.0" targetFramework="net452" />
+  <package id="Microsoft.Azure.Management.DataLake.Analytics" version="3.1.2-preview" targetFramework="net452" />
+  <package id="Microsoft.Rest.ClientRuntime" version="2.3.10" targetFramework="net452" />


@ro-joowan similar comment about remove the new references in this file. These references can be found in the packages.config file in the Commands.Common.ResourceManager project.

Done - thanks!

cormacpayne · 2017-10-18T00:39:16Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/Commands.DataLakeStore.csproj

+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.DataLake.Store.2.3.0-preview\lib\net452\Microsoft.Azure.Management.DataLake.Store.dll</HintPath>
+      <Private>True</Private>
+    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">


@ro-joowan same comment about removing the references in this file

Done - thanks!

cormacpayne · 2017-10-18T00:39:42Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/packages.config

@@ -1,4 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
-  <package id="Microsoft.Azure.Management.DataLake.Store" version="2.2.0" targetFramework="net452" />
+  <package id="Microsoft.Azure.Management.DataLake.Store" version="2.3.0-preview" targetFramework="net452" />


@ro-joowan same comment about removing the references in this file

Done - thanks!

* Removing these from csproj files and packages.config

ro-joowan · 2017-10-18T01:05:32Z

@cormacpayne - Changes made. Thank you for that!

cormacpayne · 2017-10-18T18:34:43Z

@ro-joowan the build is failing because StaticAnalysis caught the breaking changes you are trying to make, so you will need to suppress these exception. To do so, you will need to use a text editor (Notepad, VS Code, Sublime, etc.) and open the BreakingChangeIssues.csv file found in the Build Artifacts of the job, copy the contents (except for the header) and append them to the end of tools/StaticAnalysis/Exceptions/BreakingChangeIssues.csv using a text editor. Let me know if you have any questions about this.

ro-joowan · 2017-10-18T23:31:41Z

@cormacpayne The build passed after making the update to BreakingChangeIssues.csv. Thanks!

ro-joowan requested a review from cormacpayne October 5, 2017 03:07

This was referenced Oct 5, 2017

[ADLA - ADLS] - [SessionRecords] - Object refactoring #4719

Closed

[NOT FOR IGNITE] [ADLA - ADLS] - Object refactoring #4594

Closed

azuresdkci assigned markcowl Oct 5, 2017

azuresdkci added the needs-review label Oct 5, 2017

markcowl assigned cormacpayne and unassigned markcowl Oct 5, 2017

cormacpayne added needs-revision and removed needs-review labels Oct 6, 2017

cormacpayne assigned ro-joowan and unassigned cormacpayne Oct 6, 2017

markcowl added the Do Not Merge 🚫 label Oct 6, 2017

ro-joowan added 4 commits October 9, 2017 11:32

Revising changelog to note the Obsolete tag of the Basic objects

efaf9f7

ADLA SessionRecords

57c95cc

ADLS SessionRecords

f4bec0a

ro-joowan force-pushed the object-refactor-2 branch from 6c07242 to f4bec0a Compare October 9, 2017 18:33

Revising changelog to note the breaking change

b4ead11

cormacpayne suggested changes Oct 18, 2017

View reviewed changes

cormacpayne removed the Do Not Merge 🚫 label Oct 18, 2017

Removing ClientRuntime, ClientRuntime.Azure, and Newtonsoft.Json

4885905

* Removing these from csproj files and packages.config

Adding breaking changes to BreakingChangeIssues.csv

f161741

cormacpayne approved these changes Oct 20, 2017

View reviewed changes

cormacpayne merged commit de907d1 into Azure:preview Oct 20, 2017

ro-joowan deleted the object-refactor-2 branch October 23, 2017 19:14

ro-joowan mentioned this pull request Oct 30, 2017

[ADLA - ADLS] - Updating help markdown OUTPUTS #4886

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADLA - ADLS] - Object refactoring #4718

[ADLA - ADLS] - Object refactoring #4718

ro-joowan commented Oct 5, 2017 •

edited by idear1203

Loading

cormacpayne commented Oct 6, 2017

ro-joowan commented Oct 9, 2017

ro-joowan commented Oct 15, 2017

cormacpayne commented Oct 18, 2017

cormacpayne left a comment

cormacpayne Oct 18, 2017

ro-joowan Oct 18, 2017

cormacpayne Oct 18, 2017

ro-joowan Oct 18, 2017

cormacpayne Oct 18, 2017

ro-joowan Oct 18, 2017

cormacpayne Oct 18, 2017

ro-joowan Oct 18, 2017

ro-joowan commented Oct 18, 2017

cormacpayne commented Oct 18, 2017

ro-joowan commented Oct 18, 2017 •

edited

Loading

[ADLA - ADLS] - Object refactoring #4718

[ADLA - ADLS] - Object refactoring #4718

Conversation

ro-joowan commented Oct 5, 2017 • edited by idear1203 Loading

Description

cormacpayne commented Oct 6, 2017

ro-joowan commented Oct 9, 2017

ro-joowan commented Oct 15, 2017

cormacpayne commented Oct 18, 2017

cormacpayne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro-joowan commented Oct 18, 2017

cormacpayne commented Oct 18, 2017

ro-joowan commented Oct 18, 2017 • edited Loading

ro-joowan commented Oct 5, 2017 •

edited by idear1203

Loading

ro-joowan commented Oct 18, 2017 •

edited

Loading