[Maps] [Hub Generated] Review request for Weather to add version preview/1.0 #15532
Conversation
|
Hi, @john35452 Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected] |
|
[Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks. |
Swagger Validation Report
|
Swagger Generation Artifacts
|
john35452
added
DoNotMerge
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
5cf0573 to
0005144
Compare
|
NewApiVersionRequired reason: |
|
Hi @john35452, Your PR has some issues. Please fix the CI sequentially by following the order of
|
openapi-pipeline-app
bot
added
CI-FixRequiredOnFailure
and removed
CI-FixRequiredOnFailure
labels
Swagger Validation Report
|
| Rule | Message |
|---|---|
|
The new version is adding a path that was not found in the old version. New: Weather/preview/1.0/weather.json#L711:5 |
|
The new version is adding a path that was not found in the old version. New: Weather/preview/1.0/weather.json#L744:5 |
|
The new version is adding a path that was not found in the old version. New: Weather/preview/1.0/weather.json#L789:5 |
|
The new version is adding a path that was not found in the old version. New: Weather/preview/1.0/weather.json#L847:5 |
️️✔️LintDiff succeeded [Detail] [Expand]
Validation passes for LintDiff.
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️~[Staging] ApiReadinessCheck succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️Cross-Version Breaking Changes succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️️✔️SDK Track2 Validation succeeded [Detail] [Expand]
Validation passes for SDKTrack2Validation
- The following tags are being changed in this PR
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
7e93588 to
7eea9ea
Compare
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
34676bf to
cbe1f88
Compare
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
cbe1f88 to
236c14d
Compare
anuchandy
changed the title
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
7e83f06 to
ca891d1
Compare
john35452
force-pushed
the
dev-maps-Weather-Tropical
branch
from
5cf7e6d to
95d5624
Compare
| "operationId": "Weather_StormLocations", | ||
| "x-ms-examples": { | ||
| "StormLocations": { | ||
| "$ref": "./examples/GetStormLocations.json" |
There was a problem hiding this comment.
I can't seem to find the example API call at https://review.docs.microsoft.com/en-us/rest/api/documentation-preview/weather/storm-locations in this Review view , but that example doesn’t work. I get a "govId is missing or empty" error.
However, AccuWeather’s docs have a working example of https://api.accuweather.com/tropical/v1/gov/storms/2018/NP/26/forecasts?apikey=YourKey. I tried that example using our API and it doesn’t work for some reason. What exact AccuWeather API is this API using and do we have a working example that we can use instead?
There was a problem hiding this comment.
@john35452 any idea on this example? Doesn't work for me.
| "description": "**Get government-issued Storms**\n\n**Applies to**: S0 and S1 pricing tiers.\n\nSearch government-issued storms by year, basin ID, and government ID.", | ||
| "operationId": "Weather_StormSearch", | ||
| "x-ms-examples": { | ||
| "StormSearch": { |
There was a problem hiding this comment.
For consistency with the other Weather APIs, shouldn’t we refer to this API as ‘Get Storm Search’ instead of ‘Storm Search’?
| }, | ||
| "/weather/tropical/storms/forecasts/{format}": { | ||
| "get": { | ||
| "description": "**Government-issued forecasts**\n\n**Applies to**: S0 and S1 pricing tiers.\n\nGet all Government-issued forecasts.", |
There was a problem hiding this comment.
For consistency, please...
Change:
Government-issued forecasts
To:
Get Storm Forecasts
| }, | ||
| "/weather/tropical/storms/locations/{format}": { | ||
| "get": { | ||
| "description": "**Locations for an individual government-issued storm**\n\n**Applies to**: S0 and S1 pricing tiers.\n\nGet locations of an individual government-issued storm.", |
There was a problem hiding this comment.
For consistency please...
Change:
Locations for an individual government-issued storm
To:
Get Storm Locations
| }, | ||
| "/weather/tropical/storms/{format}": { | ||
| "get": { | ||
| "description": "**Get government-issued Storms**\n\n**Applies to**: S0 and S1 pricing tiers.\n\nSearch government-issued storms by year, basin ID, and government ID.", |
There was a problem hiding this comment.
For consistency please...
Change:
Get government-issued Storms
To:
Get Storm Search
| @@ -0,0 +1,9787 @@ | |||
| { | |||
There was a problem hiding this comment.
@john35452 I still get the same error as before with the example, any idea why? https://atlas.microsoft.com/weather/tropical/storms/forecasts/json?api-version=1.0&year=2021&basinId=NP&governmentId=2
| }, | ||
| "/weather/tropical/storms/forecasts/{format}": { | ||
| "get": { | ||
| "description": "**Get Storm Forecast**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all Government-issued tropical storm forecasts. Information about the forecasted tropical storms includes, location, status, date the forecast was created, window, wind speed and wind radii.", |
There was a problem hiding this comment.
| "description": "**Get Storm Forecast**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all Government-issued tropical storm forecasts. Information about the forecasted tropical storms includes, location, status, date the forecast was created, window, wind speed and wind radii.", | |
| "description": "**Get Storm Forecast**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all government-issued tropical storm forecasts. Information about the forecasted tropical storms includes, location, status, date the forecast was created, window, wind speed and wind radii.", |
| @@ -0,0 +1,5108 @@ | |||
| { | |||
There was a problem hiding this comment.
I think this example response is excessively long, which will cause customers to have scroll quite a bit to see the documentation below it (not a great experience). By simply changing 'govId=2' to 'govId=1' it returns a smaller response that I think would be a better example for the documentation. Any chance we can update with the example request/response to use the 'govId=1' example?
There was a problem hiding this comment.
I have updated the examples. Please help me to review them again.
|
Hi @john35452, Your PR has some issues. Please fix the CI sequentially by following the order of
|
| }, | ||
| "/weather/tropical/storms/{format}": { | ||
| "get": { | ||
| "description": "**Get Storm Search**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nSearch government-issued tropical storms by year, basin ID, and government ID. Information about the tropical storms includes, government ID, basin ID, status, year, name and if it is subtropical.", |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Search'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| }, | ||
| "/weather/tropical/storms/forecasts/{format}": { | ||
| "get": { | ||
| "description": "**Get Storm Forecasts**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all government-issued tropical storm forecasts. Information about the forecasted tropical storms includes, location, status, date the forecast was created, window, wind speed and wind radii.", |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Forecast'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| "description": "**Get Storm Forecasts**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all government-issued tropical storm forecasts. Information about the forecasted tropical storms includes, location, status, date the forecast was created, window, wind speed and wind radii.", | ||
| "operationId": "Weather_GetStormForecast", | ||
| "x-ms-examples": { | ||
| "Get Storm Forecasts": { |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Forecast'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| }, | ||
| "/weather/tropical/storms/locations/{format}": { | ||
| "get": { | ||
| "description": "**Get Storm Locations**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet locations of an individual government-issued tropical storms. Information about the tropical storms includes, location coordinates, geometry, basin ID, date, wind details and wind radii.", |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Locations'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| "description": "**Get Storm Locations**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet locations of an individual government-issued tropical storms. Information about the tropical storms includes, location coordinates, geometry, basin ID, date, wind details and wind radii.", | ||
| "operationId": "Weather_GetStormLocations", | ||
| "x-ms-examples": { | ||
| "Get Storm Locations": { |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Locations'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| "description": "**Get Storm Search**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nSearch government-issued tropical storms by year, basin ID, and government ID. Information about the tropical storms includes, government ID, basin ID, status, year, name and if it is subtropical.", | ||
| "operationId": "Weather_GetStormSearch", | ||
| "x-ms-examples": { | ||
| "Get Storm Search": { |
There was a problem hiding this comment.
Can we change this to 'Get Tropical Storm Search'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
| }, | ||
| "/weather/tropical/storms/active/{format}": { | ||
| "get": { | ||
| "description": "**Get Active Storms**\n\n**Applies to**: Gen 1 (S0 and S1) and Gen 2 pricing tiers.\n\nGet all government-issued active tropical storms. Information about the tropical storms includes, government ID, basin ID, year of origin, name and if it is subtropical.", |
There was a problem hiding this comment.
Can we change this to 'Get Active Tropical Storms'? I think the name should be more explicit, especially when it's listed with all the other Weather Services once in production, we want users to know it's tropical storms not regular storms.
|
/azp run unifiedPipeline |
|
No pipelines are associated with this pull request. |
This is a PR generated at OpenAPI Hub. You can view your work branch via this link.
Changelog
Add a changelog entry for this PR by answering the following questions:
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Otherwise your PR may be subject to ARM review requirements. Complete the following:
Check this box if any of the following apply to the PR so that label “WaitForARMFeedback” will be added automatically to begin ARM API Review. Failure to comply may result in delays to the manifest.
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit (including refactoring) and then push new changes, including version updates, in separate commits.
Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If any of the following scenarios apply to the PR, request approval from the Breaking Change Review Board as defined in the Breaking Change Policy.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.