Skip to content

Conversation

@n3hrox
Copy link

@n3hrox n3hrox commented Oct 27, 2019

  • Have you followed the guidelines in our
    Contributing document?

  • Does your PR affect documented changes or does it add new functionality
    that should be documented? If yes, have you created a PR for
    dvc.org documenting it or at
    least opened an issue for it? If so, please add a link to it.


Resolves #2611

@n3hrox
Copy link
Author

n3hrox commented Oct 27, 2019

I have no idea how to get DeepSource to pass these minor issues. Recommended usage by python docs is subprocess.run. I've even tried using os. that is already imported and used as os.system but even moving this code to other place causes issue. Any idea how I could omit that? I don't see any way to check for conda without using some kind of system call.

@n3hrox n3hrox mentioned this pull request Oct 27, 2019
@shcheklein
Copy link
Contributor

@n3hrox thanks for the PR!

It's not the requirement to fix all the deepsource complaints.

@sanketsaurav may be this #2679 (comment) can be of some interest to you and you can recommend us a better way to deal with this.

Copy link
Contributor

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

are there tests for this? can we add one?

@n3hrox
Copy link
Author

n3hrox commented Oct 27, 2019

Added test and refactored _get_conda to _is_conda. Let me know if this is sufficient.
This PR is the first part for #2672 which I would also love to do if I find some spare time


def is_conda():
try:
from .build import PKG # patched during conda package build
Copy link
Contributor

Choose a reason for hiding this comment

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

When we start including this file into other packages, we would be able to simply delete this dvc/utils/pkg.py and have build.py(or maybe pkg.py if we decide to rename it) have PKG = "something" defined and then just from .build import PKG and use it instead of get_package_manager. That would look and feel very nice. 🙂

Copy link
Author

Choose a reason for hiding this comment

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

@efiop I will rework that method in the next PR where I include patches for other builds as well ;)

Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@efiop efiop merged commit a5d317e into treeverse:master Nov 4, 2019
@n3hrox n3hrox requested a review from shcheklein November 4, 2019 20:54
@n3hrox n3hrox deleted the enhance/conda_detection branch November 4, 2019 20:54
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.

updater: detect conda

3 participants