-
Notifications
You must be signed in to change notification settings - Fork 340
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
Update HL7Monitor asset integration #877
Conversation
care/facility/api/viewsets/asset.py
Outdated
action = request.data["action"] | ||
asset: Asset = self.get_object() | ||
result = AssetClasses(asset.asset_class).handle_action(action) | ||
asset_class = AssetClasses[asset.asset_class].value(asset.meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle unknown asset class in this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error details are only returned when there is some validation error (post request body has missing/invalid parameters) with a status of 400. In the case of all other errors, only a message of "Internal server error" is returned with a status of 500. So, will there be any point in raising a separate new error? In any case, it is all inside a try-except block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally, internal server errors are reserved for errors that are unknown in nature. since these are known exceptions we try to handle as much as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a separate issue and start a separate PR if required.
@gigincg please review. |
fixes: #801
related to: ohcnetwork/care_fe#2159