-
Notifications
You must be signed in to change notification settings - Fork 67
Ianmacleod/error handling #282
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
Conversation
…ngine into ianmacleod/error_handling
… unneccessary logging errors
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.
adding this configuration because we are currently seeing error logs related to this not being properly set up, the error logs go away when this is off. ref: DataDog/dd-trace-py#5212
yixu34
left a comment
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.
Just wanted to clarify the repository field
| detail="deletion of endpoint failed, compute resources still exist.", | ||
| ) from exc | ||
| except Exception as exc: | ||
| request_id = get_request_id() |
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.
Think this is fine for now, but as we revisit self-hosting in VPCs, we'll probably want some sort of Gateway abstraction for getting the request/trace ID.
| Thrown when a user tries to specify a custom Docker image that cannot be found. | ||
| """ | ||
|
|
||
| repository: str |
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.
Just want to check that repository refers to a full image URL, minus the image tag. e.g. it shouldn't refer to an AWS ECR repo purely by its name. In other words, it shouldn't look like some_service, but <account_id>.../some_service or <some dockerhub prefix>.../some_service.
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.
Ok discussed offline, this is fine for now. If needed in the future, we can deprecate this field and come up with something new in its stead, like image_url.
Creates structured logging instead of having stack traces spread out over multiple logs.
Consolidates exceptions into one unified place.
Adds support for
request_idthat is surfaced when an uncaught exception occurs.