-
Notifications
You must be signed in to change notification settings - Fork 24
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
Save errors from failed YAML loading #118
Comments
Thanks James for kicking this off. I would prefer though to have this
functionality standardized across all handlers though.
I think probo-githubhandler should pass the raw yaml file to the
coordinator (or an empty string if it does not exist), and then the
coordinator would parse it and convert it to settings (or, report an error
- either that config is empty, or it does not parse, along with the error
text returned by loadSafe).
*DANIEL ZINKEVICH*
Developer
…___________________________
*ZIVTECH*
One South Broad St.
Ste. 1010
Philadelphia, PA 19107
(215)-922-2692
info@zivtech.com
www.zivtech.com
On Thu, Jul 20, 2017 at 11:37 AM, James Cole ***@***.***> wrote:
*Proposal for saving the error details of YAML loading steps.*
Steps in the process of processing a PR where there is a YAML load error:
1. Our webhook receives the PR
2. It is sent to GithubHandler's processRequest method
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L293>
3. processRequest tries to fetch the YAML file
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L304>
4. fetchProboYamlConfigFromGithub
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L389>
is called to get the YAML
5. Error callback is found
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L307>
6. Build up an error and post back to Github
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L320>
I propose that we also save the update sent to GH
<https://github.com/ProboCI/probo/blob/master/lib/GithubHandler.js#L315>
in the local build store. This will also provide a point to emit this error
to other services or webhooks that anyone using this project wants. The
build object here should have a steps array, where this error should be
saved. We'd just make sure that the step we save there is the same format
as other steps.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#118>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA4fIuQ6kCaKKdd9bRcArXZ053nNVnl_ks5sP3Q4gaJpZM4OeSly>
.
|
@dzink just keep in mind that this is an open source project, so any solution we have must comply with that ... the coordinator and other handlers are not open source, so our solution here is without those other projects in mind. Instead, the build updates the other services via our event bus, which internally for us means over Kafka. |
My inclination is to avoid having logic in many places. I'd very much prefer that we pass off whatever yaml file to the next piece of the puzzle. If our current implementation is to send that to the coordinator we should add the same functionality to the container manager so as to be able to bypass the coord. If the current functionality is to send it to kafka, then we should keep doing that. If we currently send it to the coord which sends it to kafka, then we should fix that. |
It goes through kafka already as part of the build object. Really, what we want here is a new build property that has startup errors in it, save those to the build object here locally, then publish that build over our eventbus like we already do. |
A more robust mechanism of catching build errors both in terms of misnamed assets, bad yaml settings and errors caused from YAML steps are in the works that will work as part of open source and non-open source. Especially as an open source web portal is developed independently |
@dzink @lliss
Proposal for saving the error details of YAML loading steps.
Steps in the process of processing a PR where there is a YAML load error:
I propose that we also save the update that was sent to GH in the local build store. This will also provide a point to emit this error via our eventbus to other services or webhooks that anyone using this project wants. The build object here should have a
steps
array, where this error should be saved. We'd just make sure that the step we save there is the same format as other steps.The text was updated successfully, but these errors were encountered: