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

[24.1] Update parcel-built visualizations to use parcel v2 #18311

Merged
merged 28 commits into from
Jun 6, 2024

Conversation

dannon
Copy link
Member

@dannon dannon commented Jun 4, 2024

No functional changes, just a migration from v1 to v2 of parcel and some misc cleanup of a couple things I noticed along the way (build artifacts, unused libs). This fixes #18259, which is due to the visualization failing to build on test due to missing dependencies. Parcel v1 requires deasync, which uses node-gyp to compile c++, roping build tools into the process. This is slow and obviously fails when a build toolchain is unavailable.

If you notice, we're using two versions 2.12.0 and 2.11.0. This is because there's an issue with parcel 2.12.0 and the inclusion of d3, which causes invalid output during bundling, so d3-based visualizations are using 2.11.0 until 2.13.0 is out, which contains a fix.

I tested many of these manually, and verified they all built, but I did not manually run each visualization yet.

This also adds a configuration option GALAXY_PLUGIN_BUILD_FAIL_ON_ERROR that, when set, will cause a client build to fail when a plugin build returns a nonzero exit code. I changed the github action to set this in the env, so we'll catch breaking plugins moving forward.

Still a few remaining tasks here; scatterplot is being a pain but making progress.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek
Copy link
Member

mvdbeek commented Jun 4, 2024

Looks like only openlayers still failing to build: https://github.com/galaxyproject/galaxy/actions/runs/9369100590/job/25792755747?pr=18311#step:6:582

@dannon
Copy link
Member Author

dannon commented Jun 4, 2024

@mvdbeek Yep, was holding off to see if those first tweaks addressed the other issues before spamming CI again :)

dannon added 25 commits June 5, 2024 18:57
2.12 due to compatibility issues with d3, which will be resolved in a
  new release of parcel (2.13, soon)
…n may be why babel wasn't autoresolving the polyfill
…WG meeting; fail fast in CI so we can catch and resolve these.
@dannon dannon force-pushed the parcel-migration branch from 1b7a7bd to 1fc6140 Compare June 5, 2024 22:58
Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

Looks like all built correctly ?

@dannon
Copy link
Member Author

dannon commented Jun 6, 2024

@mvdbeek Yeah, it's a good step forward. And now, if any viz fails to build in CI it'll fail and be very obvious.

I was working on adding more tests for other built-in visualizations, but I can follow up on that in dev.

@dannon dannon marked this pull request as ready for review June 6, 2024 13:04
@mvdbeek mvdbeek merged commit 3a3bbfb into galaxyproject:release_24.1 Jun 6, 2024
49 of 50 checks passed
Copy link

github-actions bot commented Jun 6, 2024

This PR was merged without a "kind/" label, please correct.

@itisAliRH itisAliRH deleted the parcel-migration branch June 12, 2024 12:52
@jdavcs jdavcs added this to the 24.1 milestone Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants