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

replace FileSystems with ZipInputStream for parallel execution #1483

Merged
merged 3 commits into from
Sep 21, 2021

Conversation

atty303
Copy link
Contributor

@atty303 atty303 commented Sep 21, 2021

The steps to reproduce the problem are as follows.

  1. Parallel execution is enabled.
  2. Targets that use scalaPBUnpackProto will be executed in parallel.
  3. FileSystemAlreadyExistsException will be thrown.

This is because FileSystems does not allow multiple instances of the same URI to be created.
Therefore, I have changed the implementation to use ZipInputStream.

@atty303 atty303 changed the title replace FilSystems with ZipInputStream for parallel execution replace FileSystems with ZipInputStream for parallel execution Sep 21, 2021
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Looks good to me. See my suggestion to use more mill API. Is this covered by any test?

contrib/scalapblib/src/ScalaPBModule.scala Outdated Show resolved Hide resolved
contrib/scalapblib/src/ScalaPBModule.scala Outdated Show resolved Hide resolved
@atty303
Copy link
Contributor Author

atty303 commented Sep 21, 2021

Thanks for the review!
I have fixed the points you pointed out.

Looks good to me. See my suggestion to use more mill API. Is this covered by any test?

I don't think there are any tests related to this. I have tested it working on a project at work.

Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@lefou lefou merged commit bd71c7d into com-lihaoyi:main Sep 21, 2021
@lefou lefou added this to the after 0.10.0-M2 milestone Sep 21, 2021
@atty303 atty303 deleted the scalapblib-zipinputstream branch September 26, 2021 05:47
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.

2 participants