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] Consider avoid using extra Get in some Create operations #18480

Closed
kinelski opened this issue Feb 5, 2021 · 2 comments
Closed
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor
Milestone

Comments

@kinelski
Copy link
Member

kinelski commented Feb 5, 2021

We usually perform an extra Get call at the end of our Create methods to populate our returned models. In CreateDataFeed this is necessary to populate properties that can't be determined at the client side after creation, in special the metric IDs.

Other Create methods, such as CreateHook, don't really obtain any extra information from these extra Get calls, and they're used exclusively to build the returned models. Consider setting the ID using an internal setter and removing the extra call.

Methods to consider:

  • CreateDetectionConfiguration
  • CreateAlertConfiguration
  • CreateHook
  • AddFeedback

Concern: consider whether it may be a good idea or not to set the ID of the model used as input by the user. It may be worth cloning the whole model to have separate references.

@kinelski kinelski added Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor labels Feb 5, 2021
@kinelski kinelski added this to the Backlog milestone Feb 5, 2021
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Mar 12, 2021
@ghost
Copy link

ghost commented Mar 12, 2021

Hi @kinelski. There hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by removing the no-recent-activity label. Otherwise, we'll close this out in 7 days.

@kinelski kinelski removed the no-recent-activity There has been no recent activity on this issue. label Mar 12, 2021
@kinelski kinelski modified the milestones: Backlog, [2021] May Apr 19, 2021
@kinelski kinelski added the blocking-release Blocks release label Jun 10, 2021
@kinelski kinelski modified the milestones: Backlog, [2021] July Jun 10, 2021
@kinelski
Copy link
Member Author

We decided to keep the extra Get to avoid cloning the input model and to populate default properties originally set to null, what could impact Update operations if the same model was used.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Cognitive - Metrics Advisor
Projects
None yet
Development

No branches or pull requests

1 participant