Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Potentially Breaking Change: Update ADLA catalog #3118

Merged
merged 4 commits into from
Apr 26, 2017
Merged

Potentially Breaking Change: Update ADLA catalog #3118

merged 4 commits into from
Apr 26, 2017

Conversation

begoldsm
Copy link
Contributor

@begoldsm begoldsm commented Apr 21, 2017 •
edited
Loading

Description

This change updates the ADLA catalog to do the following:

  1. Introduces the new package item type
  2. Allows for listing the following items from their parent's parent item (the database):
  • Table
  • view
  • Table valued function
  • Table statistics
  1. Switched the FileType enum to a string since this field is read only and will have new types added and changed with some regularity. Note that this is a "breaking" change but it is being made as part of the preview release which has not yet been announced/consumed. I updated the middle version to reflect this, but let me know if we require a major version update for this preview release and I will make it.

This checklist is used to make sure that common guidelines for a pull request are followed.

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.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as a link to the swagger spec, used to generate the code.
  • The project.json and AssemblyInfo.cs files have been updated with the new version of the SDK.

@markcowl
Copy link
Member

@begoldsm You may want to pull the latest from this PR: #3117

Also, please fill out the checklist

@begoldsm
Copy link
Contributor Author

@markcowl shame on me, I know better! :).
Here is the PR for the swagger update: Azure/azure-rest-api-specs#1119

@begoldsm begoldsm changed the title Update ADLA catalog Potentially Breaking Change: Update ADLA catalog Apr 21, 2017
Update to latest swagger spec for ADL
1c5bc58 
This change affects catalog functionality:
* Add new package catalog item type
* Update to allow for listing of common catalog items from a base
database:
- Table
- TableValuedFunction
- TableStatistics
- Views

Updates for tests and versions.

comment out new tests for ADL

doing this until i can run tests in my local environment.

Update tests and recordings.

Actually copying the right file to the right place.

update changelog.
@begoldsm begoldsm requested a review from shahabhijeet April 21, 2017 19:28
@begoldsm
Copy link
Contributor Author

@markcowl @shahabhijeet I have synced to the latest and squahsed my commits down and update the title, description and the checklist. Let me know if you have any questions or need anything else from me!

shahabhijeet
shahabhijeet suggested changes Apr 24, 2017
View reviewed changes
...Analytics/Management.DataLake.Analytics/Microsoft.Azure.Management.DataLake.Analytics.csproj Outdated
@@ -34,6 +34,7 @@

<PropertyGroup>
<TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
<Version>2.2.0-preview</Version>
Copy link
Member

Choose a reason for hiding this comment

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

@begoldsm let's continue using as that is being used across, but if you want to use then you don't need . Also you do not need any package related properties.
All you need is the PackageId, assemblyName, assembly description and package tags. All the remaining ones not needed.
Also remove all commented code.

src/SDKs/DataLake.Store/Management.DataLake.Store/generate.cmd
@@ -6,8 +6,8 @@
@echo off
setlocal

set accountSpecFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/0ade6547eff89bab427eb400ebc5da870f2c5bd0/arm-datalake-store/account/2016-11-01/swagger/account.json"
set filesystemSpecFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/0ade6547eff89bab427eb400ebc5da870f2c5bd0/arm-datalake-store/filesystem/2016-11-01/swagger/filesystem.json"
set accountSpecFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/12a6c2299296b6877fa1e5a03ed42752445877dc/arm-datalake-store/account/2016-11-01/swagger/account.json"
Copy link
Member

Choose a reason for hiding this comment

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

@begoldsm was this missed as part of earlier checkin for the DataLake Store?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no I just want to keep all specs at the same commit level.

Copy link
Member

Choose a reason for hiding this comment

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

@begoldsm URL, licensense agreement, generating assembly version info. All this is getting controlled from common properties. So removing them will be really make all the nuget consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shahabhijeet this has been fixed and I removed all other entries. I am also, in the next iteration, moving the package ID to the top for easier identification.

begoldsm added 3 commits April 24, 2017 14:41
Cleanup ADL after migration to dev17
45470f5 
This cleans up files that are no longer used and updates csproj files to
remove commented out sections
Added a number of null checks to ADLS client
cea9b3a 
We were getting some null reference exceptions on occasion so ensuring
none of these nested properties are null before indexing to them.
Move package ID to the top to address PR.
76e25f2
@shahabhijeet shahabhijeet merged commit 178ed88 into Azure:vs17Dev Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahabhijeet shahabhijeet shahabhijeet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@begoldsm @markcowl @shahabhijeet @msftclas