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

Add support for Quarto reporting #16

Merged

Conversation

jeancochrane
Copy link
Contributor

@jeancochrane jeancochrane commented Dec 7, 2023

This PR mirrors the changes in ccao-data/model-res-avm#62, adding support for Quarto reporting to the model pipeline via a reports/performance.qmd doc that gets rendered in a finalize step, uploaded to S3 in an upload step, and linked in the model run SNS notification.

Earlier when discussing this PR I had mentioned also mirroring the changes from ccao-data/model-res-avm#66 to extract the setup portion of each pipeline step into a separate script, but I realize now that that work depends on the dependency isolation work done in ccao-data/model-res-avm#65, so I'm going to punt that work to #17.

Closes #15.

@jeancochrane jeancochrane linked an issue Dec 7, 2023 that may be closed by this pull request
Comment on lines +240 to +245
# Upload performance report
aws.s3::put_object(
paths$output$report$local,
paths$output$report$s3
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See here for an example report generated by this workflow run.

@jeancochrane jeancochrane marked this pull request as ready for review December 7, 2023 19:48
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

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

Looks good @jeancochrane! Do you wanna also incorporate ccao-data/model-res-avm#66? Or maybe @wrridgeway can do that one?

- output/timing/model_timing.parquet:
cache: false
- output/metadata/model_metadata.parquet:
cache: false
- reports/performance.html:
Copy link
Member

Choose a reason for hiding this comment

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

It's really silly that your can't change the output directory of rendered Quarto docs 😬

@jeancochrane
Copy link
Contributor Author

Looks good @jeancochrane! Do you wanna also incorporate ccao-data/model-res-avm#66? Or maybe @wrridgeway can do that one?

@dfsnow If it's alright by you, I've included that in the scope of #17! I'm down to do it, and I'm also down to let Billy handle it; I just didn't want it to block this PR, since I figured that reaching parity in reporting is more urgent than reaching parity in the pipeline simplification.

@jeancochrane jeancochrane merged commit 9acbc33 into master Dec 8, 2023
4 checks passed
@jeancochrane jeancochrane deleted the jeancochrane/15-add-support-for-quarto-reporting branch December 8, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Quarto reporting
2 participants