Skip to content

Add portfolio-report to Gallery #72

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Add portfolio-report to Gallery #72

wants to merge 11 commits into from

Conversation

fizzboop
Copy link
Contributor

@fizzboop fizzboop commented Apr 17, 2025

Fixes posit-dev/connect#31054

@fizzboop fizzboop self-assigned this Apr 17, 2025
Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

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

I'm not able to install this bit of content and it get it working. It looks like CI is having the same issue.

I noticed a few problems that could be fixed, we can see if that gets us past that or if we have to do something with the manifest.json

@cgraham-rs
Copy link
Contributor

I'm not able to install this bit of content and it get it working. It looks like CI is having the same issue.

I'm curious if the expanded output from the integration test is at all helpful to us in helping debug the failure here.

@dotNomad
Copy link
Collaborator

I'm not able to install this bit of content and it get it working. It looks like CI is having the same issue.

I'm curious if the expanded output from the integration test is at all helpful to us in helping debug the failure here.

I find it helpful. It isn't giving me a great idea of next steps, but I think that is more a me thing. Seeing the same logs in the CI is super handy.

@fizzboop fizzboop requested a review from dotNomad April 17, 2025 20:46
@dotNomad
Copy link
Collaborator

#73 was just opened to add minimumConnectVersion to the manifest.extension block. We should wait to merge this until #73 merges, and then update the manifest.json


```{r}
# embed a table with some of the numeric data in the email
sign_formatter <- formatter("span",
Copy link
Contributor Author

@fizzboop fizzboop Apr 23, 2025

Choose a reason for hiding this comment

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

This seems to be the method that is causing failures in the tests. I'm not entirely sure why though. Any thoughts? @marcosnav @jonkeane @Lytol

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aaah looks like our test infra doesn't send emails (which makes sense, but we might need to either disable this, or enable "emailing" to a file on the ephemeral runner or ???) @cgraham-rs might have thoughts about how we might run a piece of content that attempts to email something and not break if it doesn't actually get to a real email.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can update our config to enable email functionality, but only log it to the console.

Copy link
Contributor

@cgraham-rs cgraham-rs Apr 24, 2025

Choose a reason for hiding this comment

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

Even with Connect configured for email the integration test still fails locally with other errors.

make -C ./integration preview EXTENSION_NAME=portfolio-report

tests-1    | E                       '2025/04/24 15:09:26.193685089 Installing tidyselect (1.2.1) '
tests-1    | E                       '... ',
tests-1    | E                       '2025/04/24 15:09:27.216505131 curl: HTTP 200 '
tests-1    | E                       'https://rspm-sync.rstudio.com/bin/4.4-jammy/425ca596cf79ef06c6025575f49f1e9fa02a73e13cbe47dc890045b9563e7923.tar.gz',
tests-1    | E                       '2025/04/24 15:09:28.624103465 Caching tidyselect.',
tests-1    | E                       '2025/04/24 15:09:28.625794840 Using cached tidyselect.',
tests-1    | E                       '2025/04/24 15:09:28.626075215 \tOK (downloaded binary)',
tests-1    | E                       '2025/04/24 15:09:28.626144465 Installing httr (1.4.7) ... ',
tests-1    | E                       '2025/04/24 15:09:29.582938674 curl: HTTP 200 '
tests-1    | E                       'https://rspm-sync.rstudio.com/bin/4.4-jammy/f55cde798b74dbd10e9d9ae7db860261d3af39f9fc269e8c50be0e664d05f86a.tar.gz',
tests-1    | E                       '2025/04/24 15:09:30.498543216 FAILED',
tests-1    | E                       "2025/04/24 15:09:30.507599966 Error: error testing if 'httr' "
tests-1    | E                       'can be loaded [error code 2]',
tests-1    | E                       '2025/04/24 15:09:30.507609841 /opt/R/4.4.0/lib/R/bin/R '
tests-1    | E                       '--vanilla -s -f '
tests-1    | E                       "'/opt/rstudio-connect/mnt/tmp/RtmpTqeVDG/renv-install-c2bc0f341.R'",
tests-1    | E                       '2025/04/24 15:09:30.507655925 '
tests-1    | E                       '================================================================================',
tests-1    | E                       '2025/04/24 15:09:30.507657841 ',
tests-1    | E                       '2025/04/24 15:09:30.507664550 Fatal error: cannot open file '
tests-1    | E                       "'-s': No such file or directory",
tests-1    | E                       '2025/04/24 15:09:30.507665008 ',
tests-1    | E                       '2025/04/24 15:09:30.507667341 Unable to fully restore the R '
tests-1    | E                       'packages associated with this deployment.',
tests-1    | E                       '2025/04/24 15:09:30.507667675 Please review the preceding '
tests-1    | E                       'messages to determine which package',
tests-1    | E                       '2025/04/24 15:09:30.507669591 encountered installation '
tests-1    | E                       'difficulty and the cause of the failure.',
tests-1    | E                       '2025/04/24 15:09:30.507669925 ',
tests-1    | E                       'Build error: exit status 1',
tests-1    | E                       'Stopped session pings to http://127.0.0.1:38025'],
tests-1    | E           'result': { 'data': 'An error occurred while building the content',
tests-1    | E                       'type': 'build-failed-error'}}
tests-1    | E       assert 1 == 0
tests-1    | E        +  where 1 = {'id': 'uI0SQ83Kb9t7uLr3', 'output': ['Building R Markdown document...', 'Bundle created with R version 4.4.3 (>=4.0) ...ilding the content', 'type': 'build-failed-error'}, 'finished': True, 'code': 1, 'error': 'exit status 1', 'last': 468}.error_code
tests-1    | 
tests-1    | tests/posit/connect/test_deployments.py:64: AssertionError
tests-1    | --- generated xml file: /connect-extensions/integration/reports/preview.xml ----
tests-1    | =========================== short test summary info ============================
tests-1    | FAILED tests/posit/connect/test_deployments.py::TestExtensionDeployment::test_extension_deploys
tests-1    | ======================== 1 failed in 191.04s (0:03:11) =========================
tests-1    | make: *** [Makefile:288: test] Error 1

make -C ./integration 2025.03.0 EXTENSION_NAME=portfolio-report

s-1    | E                       '2025/04/24 15:12:45.013711834 Caching isoband.',
tests-1    | E                       '2025/04/24 15:12:45.016033584 Using cached isoband.',
tests-1    | E                       '2025/04/24 15:12:45.016425417 \tOK (downloaded binary)',
tests-1    | E                       '2025/04/24 15:12:45.016559917 Installing jsonlite (2.0.0) ... ',
tests-1    | E                       '2025/04/24 15:12:45.987178001 curl: HTTP 200 '
tests-1    | E                       'https://rspm-sync.rstudio.com/bin/4.4-jammy/5a7cabc04c24b14775d755b72d680969aab74e20b582a918a2d1db3566e887b2.tar.gz',
tests-1    | E                       '2025/04/24 15:12:46.226613418 Error: Command failed (132)',
tests-1    | E                       '2025/04/24 15:12:46.226628126 ',
tests-1    | E                       '2025/04/24 15:12:46.226661584 Failed to run system command:',
tests-1    | E                       '2025/04/24 15:12:46.226662584 ',
tests-1    | E                       "2025/04/24 15:12:46.226665751 \t'/opt/R/4.4.0/lib/R/bin/R' "
tests-1    | E                       '--vanilla CMD INSTALL --preclean '
tests-1    | E                       "'/opt/rstudio-connect/mnt/tmp/RtmpPAAmuP/packrat-install-c214e5ea21/jsonlite' "
tests-1    | E                       "--library='/opt/rstudio-connect/mnt/app/packrat/lib/x86_64-pc-linux-gnu/4.4.0' "
tests-1    | E                       '--install-tests --no-docs --no-multiarch --no-demo ',
tests-1    | E                       '2025/04/24 15:12:46.226666459 ',
tests-1    | E                       '2025/04/24 15:12:46.226669209 The command failed with output:',
tests-1    | E                       '2025/04/24 15:12:46.226669501 Illegal instruction',
tests-1    | E                       '2025/04/24 15:12:46.226671959 ',
tests-1    | E                       '2025/04/24 15:12:46.226672376 Unable to fully restore the R '
tests-1    | E                       'packages associated with this deployment.',
tests-1    | E                       '2025/04/24 15:12:46.226676168 Please review the preceding '
tests-1    | E                       'messages to determine which package',
tests-1    | E                       '2025/04/24 15:12:46.226677001 encountered installation '
tests-1    | E                       'difficulty and the cause of the failure.',
tests-1    | E                       '2025/04/24 15:12:46.226679918 ',
tests-1    | E                       'Build error: exit status 1',
tests-1    | E                       'Stopped session pings to http://127.0.0.1:35319'],
tests-1    | E           'result': { 'data': 'An error occurred while building the content',
tests-1    | E                       'type': 'build-failed-error'}}
tests-1    | E       assert 1 == 0
tests-1    | E        +  where 1 = {'id': 'uf50x7r4DPT1HG1t', 'output': ['Building R Markdown document...', 'Bundle created with R version 4.4.3 (>=4.0) ...ilding the content', 'type': 'build-failed-error'}, 'finished': True, 'code': 1, 'error': 'exit status 1', 'last': 185}.error_code
tests-1    | 
tests-1    | tests/posit/connect/test_deployments.py:64: AssertionError
tests-1    | -- generated xml file: /connect-extensions/integration/reports/2025.03.0.xml ---
tests-1    | =========================== short test summary info ============================
tests-1    | FAILED tests/posit/connect/test_deployments.py::TestExtensionDeployment::test_extension_deploys
tests-1    | ============================== 1 failed in 57.08s ==============================
tests-1    | make: *** [Makefile:288: test] Error 1

On old Dogfood it does successfully deploy, but fails to run.

2025/04/24 11:08:56 AM:
processing file: portfolio-report-email.Rmd
2025/04/24 11:08:56 AM:
1/3                  
2025/04/24 11:08:56 AM:
2/3 [unnamed-chunk-1]
2025/04/24 11:08:56 AM:
Error in `formatter()`:
2025/04/24 11:08:56 AM:
! could not find function "formatter"
2025/04/24 11:08:56 AM:
2025/04/24 11:08:56 AM:
Quitting from portfolio-report-email.Rmd:10-25 [unnamed-chunk-1]
2025/04/24 11:08:56 AM:
Execution halted

Copy link
Contributor

@cgraham-rs cgraham-rs Apr 24, 2025

Choose a reason for hiding this comment

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

Sorry, looking at the original error in the PR workflow it seems that the problem is not related to Connect not being configured for email, but the same formatter error seen on Dogfood (see log above).

I believe the email error that follows the content error is simply because Connect tries to email error reports for content issues. That is definitely because our config does not have email enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

One more data point: the actual example from the Connect repo can successfully deploy on Dogfood and run successfully.

I did note the environment restore took 13 minutes. We have a variety of timeouts that will compete with long deploys and may need revisiting. and our integration test timeout is currently 15 minutes so that may have to increase.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fizzboop email support for the integration test Connect container has been merged. Before making any more code changes can we merge main here and re-run CI so we can validate there is no longer an email-related error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cgraham-rs: After merging main, the formatter method still seems to be causing issues. 😞

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, I did not expect it to fix the formatter error but instead to fix the email error. I'll need to dig a little more into the Connect email functionality.

tests-1    | E                       '2025/04/29 14:50:53.727332283 Execution halted',
tests-1    | E                       'Unable to render the deployed content: Rendering exited '
tests-1    | E                       'abnormally: exit status 1',
tests-1    | E                       'Failure emailing the error: The system has not been configured '
tests-1    | E                       'to send email',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, gotcha.

"platform": "4.4.3",
"metadata": {
"appmode": "rmd-static",
"primary_rmd": "portfolio-report-email.Rmd",

Choose a reason for hiding this comment

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

Suggested change
"primary_rmd": "portfolio-report-email.Rmd",
"primary_rmd": "portfolio-report.Rmd",

@fizzboop - this may be the root of the problem.

Copy link
Contributor Author

@fizzboop fizzboop Apr 29, 2025

Choose a reason for hiding this comment

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

Ah, I see. Thank you for the suggestion! I'll make a note of this to the stock-report ticket since it uses the formatter. I am curious as to why the manifest.json assigned the email R Markdown as the primary_rmd value instead of the report.

Choose a reason for hiding this comment

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

I'm not sure why the email Rmd was chosen. I'm mostly OOO this week, but @toph-allen can help you understand how the manifest was generated (assuming it was done with rsconnect). It's possible that there's an issue in rsconnect or we needed to provide an argument when generating the manifest.

Copy link
Contributor Author

@fizzboop fizzboop Apr 30, 2025

Choose a reason for hiding this comment

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

When re-creating the manifest, I did see that it was selecting the portfolio-report-email.Rmd as the primary_rmd. We select the first file which satisfies the check in the directory as the primary_rmd. This doesn't seem like a bug since there is a method to specifically select the appropriate file like so:

rsconnect::writeManifest(appPrimaryDoc = "portfolio-report.Rmd")

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

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

The manifest.json needs a newer version of R with the dependencies listed, everything else looks great 🎉

@fizzboop fizzboop requested a review from dotNomad April 30, 2025 19:59
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.

5 participants