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

Fix pre_commit version to avoid exception errors #463

Merged
1 commit merged into from
Jan 21, 2019
Merged

Fix pre_commit version to avoid exception errors #463

1 commit merged into from
Jan 21, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jan 17, 2019

When updating the conda garage environment, pre_commit gets updated to
v1.14.2, which throws the following exception when trying to push a
commit to the remote repository:

Traceback (most recent call last):
File "/home/aigonzal/miniconda2/envs/garage/bin/pre-commit", line 7,
in
from pre_commit.main import main
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/main.py", line 12, in
from pre_commit.commands.autoupdate import autoupdate
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/commands/autoupdate.py", line 14, in
from pre_commit.clientlib import CONFIG_SCHEMA
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/clientlib.py", line 169, in
for hook_id, values in _meta
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/clientlib.py", line 170, in
for key, value in values
AttributeError: module 'cfgv' has no attribute 'ConditionalOptional'
error: failed to push some refs to
'https://github.com/rlworkgroup/garage.git'

Fixing it to v.1.14.0 solves the issue.

@ghost ghost added the bug Something isn't working label Jan 17, 2019
@ghost ghost self-assigned this Jan 17, 2019
@ghost ghost requested review from gautams3, CatherineSue and jonashen January 17, 2019 19:20
@ghost ghost self-requested a review as a code owner January 17, 2019 19:20
@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #463 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
+ Coverage    64.7%   64.75%   +0.04%     
==========================================
  Files         229      229              
  Lines       14341    14341              
==========================================
+ Hits         9280     9287       +7     
+ Misses       5061     5054       -7
Impacted Files Coverage Δ
.../theano/optimizers/conjugate_gradient_optimizer.py 73.41% <0%> (+1.26%) ⬆️
garage/envs/mujoco/gather/gather_env.py 57.91% <0%> (+1.34%) ⬆️
garage/tf/optimizers/first_order_optimizer.py 81.94% <0%> (+1.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 582c3c5...86b2d22. Read the comment docs.

@coveralls
Copy link

coveralls commented Jan 17, 2019

Coverage Status

Coverage increased (+0.02%) to 65.174% when pulling 86b2d22 on fix_precommit into 582c3c5 on master.

Copy link
Member

@ryanjulian ryanjulian left a comment

Choose a reason for hiding this comment

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

Is there a fix which won't involve a version lock? What is the root cause?

When updating the conda garage environment, pre_commit gets updated to
v1.14.2, which throws the following exception when trying to push a
commit to the remote repository:

Traceback (most recent call last):
File "/home/aigonzal/miniconda2/envs/garage/bin/pre-commit", line 7,
in <module>
    from pre_commit.main import main
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/main.py", line 12, in <module>
    from pre_commit.commands.autoupdate import autoupdate
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/commands/autoupdate.py", line 14, in <module>
    from pre_commit.clientlib import CONFIG_SCHEMA
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/clientlib.py", line 169, in <module>
    for hook_id, values in _meta
File "/home/aigonzal/miniconda2/envs/garage/lib/python3.6/site-
packages/pre_commit/clientlib.py", line 170, in <listcomp>
    for key, value in values
AttributeError: module 'cfgv' has no attribute 'ConditionalOptional'
error: failed to push some refs to
'https://github.com/rlworkgroup/garage.git'

Fixing it to v.1.14.0 solves the issue.
@ghost
Copy link
Author

ghost commented Jan 19, 2019

@ryanjulian so far the problem is that pre-commit needs cfgv>=1.4.0, but cfgv 1.10 gets installed. I ran pipdeptree to check if there's conflicting dependencies that keep cfgv downgraded, but there's not other packages except for pre-commit using cfgv. I noticed that pre-commit is installed as a primary conda dependency instead of a pip dependency, so if we move pre-commit from conda to pip, pip does the right job and gets pre-commit updated with the right cfgv version.

@ryanjulian
Copy link
Member

Does moving it to pip cause any other problems? I recall we put it in conda deps for a reason.

@naeioi
Copy link
Member

naeioi commented Jan 19, 2019

@jonashen Was there a reason to make pre-commit a conda dep rather than pip dep?

@jonashen
Copy link
Member

That's just how we installed it originally I think, I don't remember.

@ghost
Copy link
Author

ghost commented Jan 19, 2019

This is the commit where Jonathon pushed the change, but nothing is mentioned about why using conda instead of pip:
09c9982
I'm clueless about what can go wrong if we move the installation of pre-commit to pip, but we can try this small change and if some different error pops out in the future, we can revert it back to conda installation and explicitly install cfgv>=1.4.0.

@ryanjulian
Copy link
Member

please submit and let us know how it goes.

@ghost ghost merged commit 2a79567 into master Jan 21, 2019
@ryanjulian ryanjulian deleted the fix_precommit branch January 21, 2019 22:37
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants