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

Refactor/11 convert to package #17

Closed
wants to merge 135 commits into from

Conversation

AlexAxthelm
Copy link
Contributor

@AlexAxthelm AlexAxthelm commented May 29, 2024

  • Working Docker build (local)
  • Working Docker build (Actions)
  • Working Test (Actions)
  • Specify Indices as their own path (don't need rest of pacta-data)
  • Calling Function as main entry point
  • R Actions
  • Major subfunctions
  • Manifest Export Add Manifest Export #20
  • Generate Results if needed To be done workflow.pacta.webapp
  • Call JSON Args
  • Deal with git warnings in create_interactive_report Nonblocking, can deal with in that repo.

Closes #11

Depends On RMI-PACTA/pacta.portfolio.report#78

Copy link

github-actions bot commented May 29, 2024

Docker build status

commit_time git_sha image
2024-06-04T15:29:30Z e778ffc ghcr.io/rmi-pacta/workflow.pacta.report:pr-17

@AlexAxthelm AlexAxthelm requested a review from jdhoffa June 3, 2024 14:45
jdhoffa
jdhoffa previously approved these changes Jun 3, 2024
@AlexAxthelm AlexAxthelm dismissed jdhoffa’s stale review June 3, 2024 15:00

The merge-base changed after approval.

@AlexAxthelm AlexAxthelm requested a review from jdhoffa June 3, 2024 15:01
jdhoffa
jdhoffa previously approved these changes Jun 3, 2024
jdhoffa
jdhoffa previously approved these changes Jun 3, 2024
@AlexAxthelm AlexAxthelm dismissed jdhoffa’s stale review June 3, 2024 15:12

The merge-base changed after approval.

Comment on lines 117 to 128
--network none \
--env LOG_LEVEL=TRACE \
--env ANALYSIS_OUTPUT_DIR="/mnt/analysis_output_dir" \
--env BENCHMARKS_DIR="/mnt/benchmarks_dir" \
--env REPORT_OUTPUT_DIR="/mnt/report_output_dir" \
--env SUMMARY_OUTPUT_DIR="/mnt/summary_output_dir" \
--mount type=bind,readonly,source=${WORKSPACE}/${BENCHMARKS_DIR},target=/mnt/benchmarks_dir \
--mount type=bind,readonly,source=${WORKSPACE}/${ANALYSIS_OUTPUT_DIR},target=/mnt/analysis_output_dir \
--mount type=bind,source=${WORKSPACE}/${REPORT_OUTPUT_DIR},target=/mnt/report_output_dir \
--mount type=bind,source=${WORKSPACE}/${SUMMARY_OUTPUT_DIR},target=/mnt/summary_output_dir \
$FULL_IMAGE_NAME \
$PARAMETERS
Copy link
Member

Choose a reason for hiding this comment

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

nit: indentation

Dockerfile Outdated
ARG DEBCONF_NOWARNINGS="yes"
LABEL org.opencontainers.image.title="workflow.pacta.report"
LABEL org.opencontainers.image.vendor="RMI"
LABEL org.opencontainers.image.base.name="ghcr.io/rmi-pacta/workflow.pacta:main"
Copy link
Member

Choose a reason for hiding this comment

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

nit: probably should match base defined above docker.io/rocker/r-ver:4.3.1 AS base

Dockerfile Outdated Show resolved Hide resolved
pacta.portfolio.utils::quit_if_no_pacta_relevant_data(total_portfolio)
} else {
log_warn("file \"{total_portfolio_path}\" does not exist.")
warning("This is weird... the `total_portfolio.rds` file does not exist in the `30_Processed_inputs` directory.")
Copy link
Member

Choose a reason for hiding this comment

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

nit: does 30_Processed_inputs still exist in this context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not.

README.md Outdated Show resolved Hide resolved
@AlexAxthelm AlexAxthelm self-assigned this Jun 10, 2024
@AlexAxthelm
Copy link
Contributor Author

Closing since something is messed up with the approvals, will reopen

@AlexAxthelm AlexAxthelm deleted the refactor/11-convert-to-package branch June 11, 2024 11:25
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.

Convert to package structure
3 participants