fix url parsing #9844
fix url parsing #9844
Conversation
tests/models/test_match_spec.py
Outdated
|
|
||
|
|
||
| def test_dist_str(self): | ||
| m1 = MatchSpec.from_dist_str("anaconda/{}::python-3.6.6-0.tar.bz2".format(context.subdir)) |
There was a problem hiding this comment.
When using .format() and positional arguments, best practice is to enumerate each position. So
"anaconda/{0}::python-3.6.6-0.tar.bz2".format(context.subdir)
One reason is that it has a substantial speed advantage, approaching the speed of the older % string composition. The performance obviously doesn't matter here, but it can make a difference in a tight loop.
| parts = {} | ||
| if dist_str.endswith(CONDA_PACKAGE_EXTENSION_V1): | ||
| dist_str = dist_str[:-len(CONDA_PACKAGE_EXTENSION_V1)] | ||
| if '::' in dist_str: |
There was a problem hiding this comment.
I guess after line 182 we should start adding in
if dist_str.endswith(CONDA_PACKAGE_EXTENSION_V2):
dist_str = dist_str[:-len(CONDA_PACKAGE_EXTENSION_V2)]
There was a problem hiding this comment.
Although another best practice is to use string slices instead of .endswith(), again for performance reasons. See comment at #9812 (comment).
kalefranz
left a comment
There was a problem hiding this comment.
Added one more comment, and still have one other comment we should address.
conda/models/match_spec.py
Outdated
| subdir = None | ||
| parts.update({ | ||
| 'channel': channel_str, | ||
| 'subdir': subdir, |
There was a problem hiding this comment.
I'm a little nervous here about adding parts["subdir"] = None, and I don't think we need to. Instead of
parts.update({
'channel': channel_str,
'subdir': subdir,
})let's just do
parts["channel"] = channel_str
if subdir:
parts["subdir"] = subdirThere was a problem hiding this comment.
Sounds good, I made that change
|
Also now you can rebase against master. |
kalefranz
left a comment
There was a problem hiding this comment.
Looks like test failure is just appveyor timeout. Everything looked to pass in what the appveyor run got to.
|
Hi there, thank you for your contribution to Conda! This pull request has been automatically locked since it has not had recent activity after it was closed. Please open a new issue or pull request if needed. |
This fixes #9814
Instead of
splitwe usersplitto correctly parse a url. We also change the max split to 1 instead of 2