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

Add dramatically improved queued installation #3209

Closed
wants to merge 26 commits into from

Conversation

techalchemy
Copy link
Member

Add dramatically improved queued installation

  • Use queues and parallelized installation
  • Better UI/UX -- progress bar moves gradually as items are moved
    on and off the queue
  • Queue is handled by item instead of in massive batches
  • TODO: Call out when task is done from the install function?

/cc @frostming @jxltom @uranusjr

Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
- Implement improvements and bugfixes in codebase
- Remote archives will now resolve properly

Signed-off-by: Dan Ryan <dan@danryan.co>
- Specific construct for isolationg operations

Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
- Use queues and parallelized installation
- Better UI/UX -- progress bar moves gradually as items are moved
  on and off the queue
- Queue is handled by item instead of in massive batches
- TODO: Call out when task is done from the install function?

Signed-off-by: Dan Ryan <dan@danryan.co>
@techalchemy techalchemy added Type: Enhancement 💡 This is a feature or enhancement request. Category: Performance Issue relates to performance labels Nov 11, 2018
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
@jxltom
Copy link
Contributor

jxltom commented Nov 12, 2018

I just found that when using dummyspinner, sp.text will show in the final output.

For example,

sp.text = to_native_string("Locking VCS Dependencies...")
will outout followings.

➜  ✗ pipenv lock
Locking [dev-packages] dependencies...
Locking [packages] dependencies...
Locking VCS Dependencies...Updated Pipfile.lock (637151)!

@techalchemy
Copy link
Member Author

That's because the dummy spinner is not a real spinner, it only writes things to stderr

@jxltom
Copy link
Contributor

jxltom commented Nov 12, 2018

Is it better to just disable output things when use dummy spinner?

The above output looks a bit redundant when users disable spinner by PIPENV_NOSPIN.

pipenv/project.py Outdated Show resolved Hide resolved
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
@techalchemy
Copy link
Member Author

Thanks for the reviews, I cleaned up everything you guys caught and a bunch of other things also. Just cut releases of pythonfinder, requirementslib, and vistir.

Now if this builds I can start merging for the last release

Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
- Fix virtualenv
- Update pythonfinder

Signed-off-by: Dan Ryan <dan@danryan.co>
Signed-off-by: Dan Ryan <dan@danryan.co>
new_order = []
target = os.path.normcase(os.path.normpath(os.path.abspath(path)))
path_map = {
os.path.normcase(os.path.normpath(os.path.abspath(pth))): pth
Copy link
Contributor

@jxltom jxltom Nov 13, 2018

Choose a reason for hiding this comment

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

The pth may be a PosixPath type, then abspath will raise AttributeError: 'PosixPath' object has no attribute 'startswith'

Copy link
Member Author

Choose a reason for hiding this comment

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

Just saw this. Pushed a fix on the other branch. Thanks for keeping track, I’m planning to annotate these non-pipenv codebases

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

attrs==18.2.0
distlib==0.2.8
packaging==18.0
pyparsing==2.2.2
pytoml==0.1.19
plette==0.2.2
tomlkit==0.4.6
tomlkit==0.5.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't update this line or otherwise vendoring build will fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

I undid this on the other branch since I need that to pass first anyway

techalchemy added a commit that referenced this pull request Nov 13, 2018
…tenance/merge-3191-3196-3209

- Merge #3191, #3196, and #3209
- Closes #3191
- Closes #3196
- Closes #3214
- Closes #3209

Signed-off-by: Dan Ryan <dan@danryan.co>
@frostming frostming deleted the feature/improved-async-installer branch November 5, 2020 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Performance Issue relates to performance Type: Enhancement 💡 This is a feature or enhancement request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants