Skip to content

Introduce 'top' filter to Pricesheet API#1977

Merged
lmazuel merged 3 commits intoAzure:restapi_auto_consumption/resource-managerfrom
shbha:master
Feb 14, 2018
Merged

Introduce 'top' filter to Pricesheet API#1977
lmazuel merged 3 commits intoAzure:restapi_auto_consumption/resource-managerfrom
shbha:master

Conversation

@shbha
Copy link

@shbha shbha commented Feb 14, 2018

No description provided.

# Conflicts:
#	azure-mgmt-consumption/tests/test_mgmt_consumption.py
…est.

All files besides the following are related to line-endings. Since AutoRest generated the files, its left as is.
price_sheet_operations.py
test_mgmt_consumption.test_consumption_subscription_price_sheet.yaml
consumption/version.py was reset to 2.0.0 (different than what AutoRest generated)
@codecov-io
Copy link

codecov-io commented Feb 14, 2018

Codecov Report

Merging #1977 into master will increase coverage by <.01%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1977      +/-   ##
==========================================
+ Coverage   53.17%   53.17%   +<.01%     
==========================================
  Files        4714     4714              
  Lines      117148   117152       +4     
==========================================
+ Hits        62295    62298       +3     
- Misses      54853    54854       +1
Impacted Files Coverage Δ
...t/consumption/operations/price_sheet_operations.py 94.91% <75%> (-1.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80fc6f0...d372b7f. Read the comment docs.

@lmazuel
Copy link
Member

lmazuel commented Feb 14, 2018

Hi @shbha. Thank you for your contribution. Howerver, I can't take it as-is, since this code is auto-generated from a RestAPI meta-description.
@kjeur does pricesheet supports top as said in this PR? If yes, could you update the Swagger to integrate it?

@lmazuel lmazuel requested a review from kjeur February 14, 2018 17:17
@shbha
Copy link
Author

shbha commented Feb 14, 2018

@lmazuel - Yes, the swagger for Pricesheet API was updated\published\merged prior to this PR, and it does support top. The changes you notice in this PR are auto-generated using AutoRest based on the Azure Rest api repository. Also, you'll notice in the new recording, the request and response for pricesheet respects the top setting. Link: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/consumption/resource-manager/Microsoft.Consumption/stable/2018-01-31/consumption.json refer to ln 1010. Thank you.

@kjeur
Copy link
Contributor

kjeur commented Feb 14, 2018

@lmazuel I do see $top swagger changes here Azure/azure-rest-api-specs#2485. As swagger is already updated I think we are good for merge, LMK if any suggestions.
@shbha Thanks for taking care of this.

@lmazuel
Copy link
Member

lmazuel commented Feb 14, 2018

@shbha @kjeur seems you didn't expose on your github profile your organisations (so you don't have the "Member" tag on your contribution), so I didn't realize this was a contribution from Microsoft and not a random customer :). We have regular contributions from customers that doesn't know we use Swagger, and since you didn't describe the change @shbha , I didn't know there was a Swagger.
This is then a duplicate of #1971 (look at the Swagger PR, there was a link to PR 1971).
I take care of this today,
Thanks,

@lmazuel lmazuel changed the base branch from master to restapi_auto_consumption/resource-manager February 14, 2018 19:30
@lmazuel
Copy link
Member

lmazuel commented Feb 14, 2018

Merged to feature branch restapi_auto_consumption/resource-manager

@lmazuel lmazuel merged commit fd417eb into Azure:restapi_auto_consumption/resource-manager Feb 14, 2018
lmazuel pushed a commit that referenced this pull request Feb 21, 2018
…le values in examples. Earlier version had incorrect format for the date values. (#1999)

* Introduce 'top' filter to Pricesheet API (#1977)

* Introducing (rough) tests around Price Sheet API.

* Introduce 'top' filter to Pricesheet Api. And update the associated test.
All files besides the following are related to line-endings. Since AutoRest generated the files, its left as is.
price_sheet_operations.py
test_mgmt_consumption.test_consumption_subscription_price_sheet.yaml
consumption/version.py was reset to 2.0.0 (different than what AutoRest generated)

* Generated from cbefdc6e1ebb76efd65b14d3737cc6fe6f20d0d0

Updating $filter parameter sample values in examples. Earlier version had incorrect format for the date values.

* Update version.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants