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.

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

[MetricsAdvisor] Regenerate code from latest swagger #21495

Merged
merged 10 commits into from
Jun 2, 2021

Conversation

kinelski
Copy link
Member

@kinelski kinelski commented Jun 1, 2021

Part of #21177.

A bunch of internal renames and internal code changes since some properties are not marked as required anymore. We also removed the enum value DataFeedGranularityType.PerSecond because the service does not support it anymore.

We could also remove one of our swagger transforms.

@kinelski kinelski self-assigned this Jun 1, 2021
@kinelski kinelski added the Client This issue points to a problem in the data-plane of the library. label Jun 1, 2021
@kinelski kinelski requested a review from maririos June 1, 2021 23:09
@kinelski kinelski marked this pull request as ready for review June 1, 2021 23:09
Copy link
Member

@maririos maririos left a comment

Choose a reason for hiding this comment

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

We could also remove one of our swagger transforms.

Do you mean the credential one?

Not related to your PR:

  • With the pagination work you did in a previous PR, do you still need the pagination transforms?
  • Any change the service can add the required properties to their swagger instead of having so many transofrms?

@kinelski
Copy link
Member Author

kinelski commented Jun 2, 2021

We could also remove one of our swagger transforms.

Do you mean the credential one?

Correct. We removed the credential transform.

Not related to your PR:

* With the pagination work you did in a previous PR, do you still need the pagination transforms?

The pagination work I did is not related to the transforms we have in the swagger. That was due to a service-side change.

* Any change the service can add the required properties to their swagger instead of having so many transofrms?

Some of them may already be fixed on the swagger-side, but I need to double check. I will bring the discussion up with Johan, but I wouldn't say this is high priority right now. The service is a bit busy with GA, and changing a property from non-required => required in the swagger would not be a breaking change, so we can always fix it later if we can convince them.

@kinelski kinelski merged commit b88053d into Azure:master Jun 2, 2021
@kinelski kinelski deleted the ma-swagger branch June 2, 2021 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants