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 solutions for common errors #2396

Merged
merged 1 commit into from Jun 19, 2020
Merged

Add solutions for common errors #2396

merged 1 commit into from Jun 19, 2020

Conversation

sdispater
Copy link
Member

@sdispater sdispater commented May 8, 2020

Pull Request Check List

Resolves: #2386
Resolves: #2076
Resolves: #2054
Resolves: #1930

This PR adds a way to provide solutions for common errors that users may encounter while using Poetry.

For now, the only solution implemented is for the resolution process failures due to Python requirements incompatibilities, which is the most common source of confusion regarding the way the resolver works.

This will serve as the base implementation for future solutions that we might want to add to provide an overall better user experience.

Here is an example:

Screenshot 2020-05-08 at 11 23 41

  • Added tests for changed code.
  • Updated documentation for changed code.

@sdispater sdispater added the area/cli Related to the command line label May 8, 2020
@sdispater sdispater added this to the 1.1 milestone May 8, 2020
@sdispater sdispater added this to In progress in 1.1 via automation May 8, 2020
@sdispater sdispater requested a review from a team May 8, 2020
@sdispater sdispater added the area/ux Features and improvements related to the user experience label May 8, 2020
Copy link
Member

@abn abn left a comment

Looking forward to seeing this in action. A few minor comments.

@@ -40,7 +39,7 @@ class VersionSolver:
def __init__(
self,
root, # type: ProjectPackage
provider, # type: Provider
provider,
Copy link
Member

@abn abn May 10, 2020

Choose a reason for hiding this comment

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

Is there a reason why we are dropping the type hint here?

Copy link
Member Author

@sdispater sdispater May 11, 2020

Choose a reason for hiding this comment

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

I don't remember exactly. I thin I got a cyclic import error at some point due to this import. I can check again to see if we really need to remove it.

Copy link
Member Author

@sdispater sdispater May 22, 2020

Choose a reason for hiding this comment

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

So, I just checked and we need to remove it to avoid a cyclic import error

Copy link
Contributor

@tarkatronic tarkatronic Jun 10, 2020

Choose a reason for hiding this comment

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

mypy/typing actually has a solution for just this problem! https://mypy.readthedocs.io/en/stable/common_issues.html#import-cycles

tl;dr:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from poetry.puzzle.provider import Provider

Fixed!

poetry.lock Outdated
category = "main"
description = "Manage Python errors with ease"
marker = "python_version >= \"3.6\" and python_version < \"4.0\""
name = "crashtest"
Copy link
Member

@abn abn May 10, 2020

Choose a reason for hiding this comment

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

I can see crashtest on pypi but not on github, is this expected?

Copy link
Member Author

@sdispater sdispater May 14, 2020

Choose a reason for hiding this comment

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

It's not open source yet, I need to write a proper README for the project first. It should be ready tomorrow.

Copy link
Member Author

@sdispater sdispater May 22, 2020

Choose a reason for hiding this comment

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

@@ -0,0 +1,3 @@
from .python_requirement_incompatibility_solution_provider import (
Copy link
Member

@abn abn May 10, 2020

Choose a reason for hiding this comment

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

These modules are a mouth full :) Would be great to put them into a structure, eg poetry.solutions.providers.incompatibility.python_requreiment.

Would solutions be better placed in poetry.mixology consdiering we only really provide solutions for version solving issues or is the intent here to provide a generic mechanism?

Copy link
Member Author

@sdispater sdispater May 11, 2020

Choose a reason for hiding this comment

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

I wanted them to be as explicit as possible :-) But yeah I can see if we can reorganize a bit.

The idea is to extend the solutions beyond solver issues, for instance for configuration and validation errors.

Copy link
Member Author

@sdispater sdispater May 22, 2020

Choose a reason for hiding this comment

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

So, I reorganized a bit and now each module (in this case mixology) can provide its own solutions and providers.

This allows for a more flexible implementation.

1.1 automation moved this from In progress to Review in progress May 10, 2020
@sdispater sdispater requested a review from abn Jun 5, 2020
@sdispater sdispater merged commit ea7d9d1 into develop Jun 19, 2020
1.1 automation moved this from Review in progress to Done Jun 19, 2020
@sdispater sdispater deleted the error-solutions branch Jun 19, 2020
@sdispater sdispater mentioned this pull request Jun 26, 2020
@sdispater sdispater mentioned this pull request Jul 30, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to the command line area/ux Features and improvements related to the user experience
Projects
No open projects
1.1
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants