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

Chore: Connector puppeteer use lock only in detached mode #4091

Merged
merged 2 commits into from
Nov 10, 2020

Conversation

sarvaje
Copy link
Contributor

@sarvaje sarvaje commented Oct 30, 2020

Fix #4089

Pull request checklist

Make sure you:

For non-trivial changes, please make sure you also:

  • Added/Updated related documentation.
  • Added/Updated related tests.

Short description of the change(s)

Detach mode is used mostly for the test runner, to avoid opening multiple instances of the browser. To do so, it store in disk the browser information of the first instance opened, so the next calls can reuse that information.

Before this PR, the writing/reading of the disk were done even if it wasn't running in detach mode.

This PR disables the writings to disk if we are not in detach mode. This will save some time accessing to disk, but more important, it will solve a problem running connector-puppeteer in Azure Functions Consumption plan.

Azure Function Consumption plan doesn't allow writing in the disk, except for the tmp folder, so this change is needed if we want to complete webhintio/serverless-online-service#143

Copy link
Member

@antross antross left a comment

Choose a reason for hiding this comment

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

The changes look fine, but can you add a bit more detail explaining why this won't work in the consumption plan without this change, and why using the lock isn't necessary in that context? Thanks!

@sarvaje
Copy link
Contributor Author

sarvaje commented Nov 2, 2020

The changes look fine, but can you add a bit more detail explaining why this won't work in the consumption plan without this change, and why using the lock isn't necessary in that context? Thanks!

Do you mean in the code or in the PR?

@antross
Copy link
Member

antross commented Nov 2, 2020

The changes look fine, but can you add a bit more detail explaining why this won't work in the consumption plan without this change, and why using the lock isn't necessary in that context? Thanks!

Do you mean in the code or in the PR?

I meant in the PR description. I figure we'll keep that around as part of the squashed commit comment too. Though now that you mention it, updating the existing code comment wouldn't hurt either 😉

@sarvaje
Copy link
Contributor Author

sarvaje commented Nov 2, 2020

@antross please take another look.

Copy link
Member

@antross antross left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@sarvaje sarvaje merged commit 1561abd into webhintio:main Nov 10, 2020
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.

[Feature] Allow connector puppeteer to disable writing to disk
2 participants