Skip to content

Create example of workflow with a build step#23

Merged
dotNomad merged 1 commit intomainfrom
dotnomad/extension-with-build
Feb 27, 2025
Merged

Create example of workflow with a build step#23
dotNomad merged 1 commit intomainfrom
dotnomad/extension-with-build

Conversation

@dotNomad
Copy link
Collaborator

@dotNomad dotNomad commented Feb 24, 2025

This PR introduces a new GitHub Workflow for the Publisher Commander Center extension that illustrates how we can integrate a build job prior to the package job.

This is very similar to the workflows introduced in #20 with a few differences:

  • There are now two jobs build -> package
  • The build job does whatever it needs to do to create the extension code (in this case npm run build) and uploads an artifact for the next job to use.
  • Rather than TARing up the extension's directory it TARs up the artifact from the build job

The PR also includes some changes to the publisher-command-center extension like:

  • adding a connect-extension.toml which is required to install the extension via the TAR bundle
  • removing the index.html from the manifest.json of the extension since it isn't needed (it uses dist/index.html)

To Test

  1. Grab the publisher-command-center.tar.gz Artifact from the "Publisher Command Center Extension" workflow.
    • This can be found by clicking "Checks" above and then "Publisher Command Center Extension" on the left. Artifacts are on the bottom of the page.
  2. Install the extension using the "Add an Extension" via bundle feature on Dogfood verifying the installation and the content is working.

@dotNomad dotNomad force-pushed the dotnomad/extension-with-build branch 3 times, most recently from 9ffcc5c to cfe905b Compare February 24, 2025 21:14
@dotNomad dotNomad changed the title Add publisher-command-center extension workflow Create example of workflow with a build step Feb 24, 2025
@dotNomad dotNomad marked this pull request as ready for review February 24, 2025 21:18
@dotNomad dotNomad force-pushed the dotnomad/extension-with-build branch from 567b0f4 to 5008a18 Compare February 24, 2025 23:00
@dotNomad
Copy link
Collaborator Author

Setting this one back to a draft, the TAR file I'm creating is larger than the 10 MB limit and we should be able to get it much smaller 👀

@dotNomad dotNomad marked this pull request as draft February 24, 2025 23:05
- Remove index.html from publisher cmd manifest
- Add connect-extension.toml to the publisher-command-center extension
@dotNomad dotNomad force-pushed the dotnomad/extension-with-build branch from afde6a3 to 09657f0 Compare February 24, 2025 23:44
@dotNomad dotNomad marked this pull request as ready for review February 24, 2025 23:46
@jonkeane
Copy link
Collaborator

is larger than the 10 MB limit and we should be able to get it much smaller 👀

Where / what is this limit?

@dotNomad
Copy link
Collaborator Author

dotNomad commented Feb 25, 2025

is larger than the 10 MB limit and we should be able to get it much smaller 👀

Where / what is this limit?

It appears when trying to use a bundle higher than 10 MB.

CleanShot 2025-02-25 at 14 12 04@2x

A bit of digging: it is based on the Content Images setting MaxAppImageSize.

According to the Connect Admin Guide:

Posit Connect rejects uploads of images larger than the Applications.MaxAppImageSize configuration setting value. By default, the server accepts images up to 10MB in size.

@aronatkins
Copy link

aronatkins commented Feb 26, 2025

@dotNomad - It looks like we have incorrectly applied Applications.MaxAppImageSize to bundle uploads. The setting is intended to limit the size of THUMBNAIL uploads, not bundles.

The FileInput component uses Applications.MaxAppImageSize as a size limit for all uses of FileInput. Instead, the limit should be different for different file upload contexts. Looks like we had not uncovered that problem, yet.

Connect does not limit bundle uploads.

@jonkeane
Copy link
Collaborator

jonkeane commented Feb 26, 2025

Ha! Sneaky. And for posterity (cause I went to look): that check is in the frontend. So presumably uploading with the API would have been fine.

You're totally right Jordan that this particular extension also shouldn't be >10MB, but it's very possible that folks upload large bundles to Connect (since they might have datasets alongside them for example). But this extension definitely doesn't need to be that large so yeah we should make sure we're not pulling in extraneous files, etc.

Could we spin off fixing the FileInput check for extensions into a separate issue? We should fix it, but it's technically not critical path for the extensions work we're doing if they get installed with API call(s).

@aronatkins
Copy link

Yes. The API does not restrict bundle upload sizes.

@dotNomad
Copy link
Collaborator Author

dotNomad commented Feb 26, 2025

You're totally right Jordan that this particular extension also shouldn't be >10MB, but it's very possible that folks upload large bundles to Connect (since they might have datasets alongside them for example). But this extension definitely doesn't need to be that large so yeah we should make sure we're not pulling in extraneous files, etc.

Having the method to select only necessary files is very nice so glad to have that in. Drastically lowering the file size is helpful.

The FileInput component uses Applications.MaxAppImageSize as a size limit for all uses of FileInput. Instead, the limit should be different for different file upload contexts. Looks like we had not uncovered that problem, yet.
Could we spin off fixing the FileInput check for extensions into a separate issue? We should fix it, but it's technically not critical path for the extensions work we're doing if they get installed with API call(s).

I'll create an issue on the Connect issue board 👍

Copy link
Contributor

@fizzboop fizzboop left a comment

Choose a reason for hiding this comment

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

LGTM 🐱

@dotNomad dotNomad merged commit 2f52b68 into main Feb 27, 2025
4 checks passed
@dotNomad dotNomad deleted the dotnomad/extension-with-build branch February 27, 2025 20: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.

4 participants