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

Progress messages for writing to cache #9368

Merged
merged 12 commits into from
Nov 30, 2023
Merged

Progress messages for writing to cache #9368

merged 12 commits into from
Nov 30, 2023

Conversation

thebriando
Copy link
Contributor

↪️ Pull Request

Writing to cache on shutdown can take a long time for large graphs, which makes it looks like Parcel is hanging after hitting ctrl-c. This moves the shutdown message to Parcel.js so it logs out right away, and also adds a progress bar indicator for the blobs written to cache.

This behavior will only happen for large graphs (>5000 nodes) but this number can be changed if needed.

💻 Examples

Screen.Recording.2023-11-08.at.4.15.07.PM.mov

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@lettertwo
Copy link
Contributor

Should the progress bar rendering be the responsibility of the CLIReporter? Having the RequestTracker do it is circumventing the reporter plugins entirely.

Maybe there should be a new ReporterEvent that represents this progress, and the CLIReporter can handle those events by rendering the progress bar.

@thebriando
Copy link
Contributor Author

I ran into some problems in the unit tests with using the report api but it should work now with the change in #9396

@thebriando thebriando merged commit e2180a2 into v2 Nov 30, 2023
15 of 16 checks passed
@mischnic mischnic deleted the bdo/cache-progress branch December 2, 2023 08:17
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