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

[refactor] move code from MetricsDataController to MetricsService #2437

Merged
merged 5 commits into from
Aug 6, 2024

Conversation

pwallk
Copy link
Contributor

@pwallk pwallk commented Aug 1, 2024

What's changed?

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

@Calvin979 Calvin979 added the good first pull request Good for newcomers label Aug 1, 2024
@Calvin979 Calvin979 linked an issue Aug 5, 2024 that may be closed by this pull request
6 tasks
public MetricsData getMetricsData(Long monitorId, String metrics) {
boolean available = realTimeDataReader.isServerAvailable();
if (!available) {
throw new CommonException("real time store not available");
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this CommonException is not catched by MetricsDataController.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CommonException is caught by the GlobalExceptionHandler in Manager, making sure it is the same as before, and throwing an exception forward when it is not available


/**
*
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you help complete comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Copy link
Contributor

@Calvin979 Calvin979 left a comment

Choose a reason for hiding this comment

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

lgtm

@Calvin979 Calvin979 merged commit d6b1969 into apache:master Aug 6, 2024
3 checks passed
@pwallk pwallk deleted the refactor-MetricsDataController branch August 15, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[Task] <Move code from controller to service>
2 participants