search

RMI-PACTA / workflow.transition.monitor

This repository is used to create the docker image that runs the transition monitor platform.
Other
2 stars 0 forks source link

protect workflow from errors in executive summary? #335

Open cjyetman opened 2 months ago

cjyetman commented 2 months ago

@jdhoffa should I wrap the executive summary stuff in a tryCatch() or something to protect against errors?

AlexAxthelm commented 2 months ago

I think that's a bad idea, since if would mean that errors in the ES would fail silently in our testing pipelines (which should be breaking if something is wrong on main)

cjyetman commented 2 months ago

maybe ES should have more robust testing on that end?

AlexAxthelm commented 2 months ago

🤷 Yeah. But I don't know enough about the content to be the one to set that up.

jdhoffa commented 2 months ago

1 - I agree with @AlexAxthelm and dont think anything should be done in this repo to changing how the errors of the dependent package are handled 2 - I agree with @cjyetman and pacta.executive.suymmary absolutely needs a way more robust testing framework, but don't have anybody with capacity that can/ wants to do this right now. @cjyetman if you're interested in giving it a go then by all means, but right now there are plenty of other actual content changes we need to make to ES on a pretty tight timeline

cjyetman commented 2 months ago

It's kinda frustrating to leave this repo in a state where its entire testing process breaks if a minor error is made somewhere else that I have little to no control over just to catch any problems with another repo, but I get that we don't have much capacity to improve the situation at the moment

jdhoffa commented 2 months ago

Yeah for sure. Although adding tests to pacta.executive.summary wouldn't have caught the issue either.

The problem is that a breaking change was introduced to pacta.executive.summary (the addition of a new/ necessary function parameter), that was not being called in workflow.transition.monitor.

Unit tests don't catch breaking changes to function APIs, that's gotta be noticed and communicated by the maintainer and/or reviewer, and PRs to broken dependencies should be opened simultaneously...