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

flake8 not properly added to repo #1425

Closed
doutriaux1 opened this issue Jun 24, 2015 · 20 comments
Closed

flake8 not properly added to repo #1425

doutriaux1 opened this issue Jun 24, 2015 · 20 comments
Assignees
Milestone

Comments

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Jun 24, 2015

@dlonie @jbeezley @aashish24 this is why I dislike pip sooo much.

Behind our firewall we can't get pip tarballs (https) so I went and downloaded the tarball manually, now while installing I get:

> pip install ~/Downloads/flake8-2.4.1.tar.gz 
Unpacking /home/doutriaux1/Downloads/flake8-2.4.1.tar.gz
  Running setup.py egg_info for package from file:///home/doutriaux1/Downloads/flake8-2.4.1.tar.gz

Downloading/unpacking pyflakes<0.9,>=0.8.1 (from flake8==2.4.1)
  Cannot fetch index base URL http://pypi.python.org/simple/
  Could not find any downloads that satisfy the requirement pyflakes<0.9,>=0.8.1 (from flake8==2.4.1)
No distributions at all found for pyflakes<0.9,>=0.8.1 (from flake8==2.4.1)
Storing complete log in /home/doutriaux1/.pip/pip.log

And down the rabbit hole we go.... Plus this is NOT the latest pyflakes that they want...
Ok so I install pyflake 0.8.1 (not 0.9.2)
back to installing flake8:

> pip install ~/Downloads/flake8-2.4.1.tar.gz 
Unpacking /home/doutriaux1/Downloads/flake8-2.4.1.tar.gz
  Running setup.py egg_info for package from file:///home/doutriaux1/Downloads/flake8-2.4.1.tar.gz

Requirement already satisfied (use --upgrade to upgrade): pyflakes<0.9,>=0.8.1 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Downloading/unpacking pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 (from flake8==2.4.1)
  Cannot fetch index base URL http://pypi.python.org/simple/
  Could not find any downloads that satisfy the requirement pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 (from flake8==2.4.1)
No distributions at all found for pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 (from flake8==2.4.1)
Storing complete log in /home/doutriaux1/.pip/pip.log

AGAIN NOT the latest version... fine pep8 is now installed... trying AGAIN flake8

pip install ~/Downloads/flake8-2.4.1.tar.gz 
Unpacking /home/doutriaux1/Downloads/flake8-2.4.1.tar.gz
  Running setup.py egg_info for package from file:///home/doutriaux1/Downloads/flake8-2.4.1.tar.gz

Requirement already satisfied (use --upgrade to upgrade): pyflakes<0.9,>=0.8.1 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Requirement already satisfied (use --upgrade to upgrade): pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Downloading/unpacking mccabe<0.4,>=0.2.1 (from flake8==2.4.1)
  Cannot fetch index base URL http://pypi.python.org/simple/
  Could not find any downloads that satisfy the requirement mccabe<0.4,>=0.2.1 (from flake8==2.4.1)
No distributions at all found for mccabe<0.4,>=0.2.1 (from flake8==2.4.1)
Storing complete log in /home/doutriaux1/.pip/pip.log

And finally

pip install ~/Downloads/flake8-2.4.1.tar.gz 
Unpacking /home/doutriaux1/Downloads/flake8-2.4.1.tar.gz
  Running setup.py egg_info for package from file:///home/doutriaux1/Downloads/flake8-2.4.1.tar.gz

Requirement already satisfied (use --upgrade to upgrade): pyflakes<0.9,>=0.8.1 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Requirement already satisfied (use --upgrade to upgrade): pep8!=1.6.0,!=1.6.1,!=1.6.2,>=1.5.7 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Requirement already satisfied (use --upgrade to upgrade): mccabe<0.4,>=0.2.1 in ./install/lib/python2.7/site-packages (from flake8==2.4.1)
Installing collected packages: flake8
  Running setup.py install for flake8

    Installing flake8 script to /home/doutriaux1/build_setuptools/install/bin
Successfully installed flake8
Cleaning up...

Yeah!

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

Uploading all tarballs to LLNL so @dlonie can create all the new little pkg files... sorry...

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@aashish24 this is why we shouldn't in general approve PR within the same institution... I think we need to remove flake8 from the buyild, you whine and whine when I added ONE dependency with pep8 but now here we are...

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

So that means at the very least rebasing all of @dlonie PR sitting there for now.

@jbeezley
Copy link

@jbeezley jbeezley commented Jun 24, 2015

To be fair, pip is doing exactly what it should do. Your network is performing a man in the middle attack on all secure traffic going in and out. Pip detects that the stream has been tampered with and refuses to install the software.

Clearly no one will be successful in convincing anyone to change the network security, but have you ever tried the --cert /path/to/rootCA option of pip? Assuming the firewall rewrites the credentials correctly, this should work just fine. Another option to handle this would be to host an internal PyPI mirror with all the packages you need.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 24, 2015

@doutriaux1 Can you provide a more straightforward description of what needs to be done? I get that your institution's firewall is preventing some standard tools from working properly and are having trouble fetching packages, but I'm not familiar with the work-arounds your group uses to bypass the issue.

What additional packages/versions are needed?

I'll wait until you address @jbeezley's suggestion before moving forward as well. Addressing the security issue, using a LLNL-specific mirror, or prepopulating a local file cache with the tarballs would be a better solution IMO than further cluttering up the superbuild.

Problems introduced at the institution level are best dealt with using institution-level solutions, such as local mirrors ;)

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@jbeezley the "fetching" is not really the issue, what I DON'T like about pip is that it installs a bunch of additional software so you think you add one little pure python script and you end up with 6 or 7 additional packages some of them with C (see pyopenssl for example) and build error on other machines. I know about the cert thing, it was even part of UVCDAT install before, but it failed on so many governement system and having to document/explain this got out of control, remember we need a SMOOTH experience for the user and most of them have no idea how to use certs or where they are.
@dlonie as I just said certs is NOT the way to go (potential unknown build erros, many "secret" packages installed, cumbersomeness) the way to do it is to add the additional packages to the _deps.cmake file and create the needed _pkg.cmake/_deps.cmake/_externals.cmake files for each additional packages. I know it's a pain. Again pep8 was pure python no deps and super easy to install, do we REALLY REALLY REALLY need flake8?

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@dlonie the sources are already uploaded on LLNL server, versions are documented in my comments above.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 24, 2015

So your complaint is that some packages have dependencies? That's not really a pip problem either...

Honestly, none of these tools should be in the superbuild at all. I'm not sure why UVCDAT insists on building everything it uses, instead of making them dependencies and just checking for them at configure time. If you don't want to deal with platform specific build issues for dependencies, the way to address that is to put it on the user -- make them responsible for installing/configuring the external projects you use, and just check that they're around when building.

But if we want everything to go into the superbuild, we'll need their deps, too. I can go back and pick through your post when I get back to UVCDAT, but in the future, a simple list of missing packages/versions is much more digestible than a rant.

As for if we really need flake8 over pep8, well, neither is strictly necessary. But given the large number of issues in the UVCDAT code, the more automated linting we can do, the better. It found 100k+ issues, and that's just looking at VCS/DV3D -- so I'd say that yes, it is useful.

But, at the end of the day, I don't really care and it's not up to me. Let me know how you folks want to proceed and I'll make it happen.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jun 24, 2015

@dlonie that's the plan. In the future, we will notify users when a deps is missing and not build by UV-CDAT. I agree that we need to keep the bulild packages list short to avoid creating a giant toolkit that may turn off some folks. I don't know enough about flake8 install so cannot say much there but detecting missing system packages is something is in the plan for the next few weeks. When that happens, you will have something like this:

flake8_deps(CORE a b c SYSTEM d). SYSTEM d meaning that we don't build d and will error out if its
missing. CORE a b c meaning that we will use system first but if we don't find it, we will build a b c ourselves.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 24, 2015

@aashish24 That sounds much better, especially if most things are moved to SYSTEM.

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@aashish24 it is a requirement I believe that you can install w/o root. If you dump everything on the user where do we draw the line? system vtk?

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@dlonie I have no issue with adding dependencies it's @aashish24 that complains everytime we add some.

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

I just don't like pip because of all the "magic" it does behind, I like to know what I install. Anyway, can somebody add these little packages please since we want flake8

@doutriaux1
Copy link
Contributor Author

@doutriaux1 doutriaux1 commented Jun 24, 2015

@dlonie just for ref, here's all the noise when I asked to add pep8: #1290

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 24, 2015

The line is best drawn where it makes sense. If we continue to need a modified version of VTK, then we should keep it in the superbuild, or even just point users at it and say 'it needs this patch'. I see the value of bundling these things for a package, but forcing developers to keep a separate version of system or commonly shared libraries is just bizarre.

But to the point, the line is where it makes since: If we don't modify a package and it is commonly available, it shouldn't go in the superbuild.

And re: your latest comment, I still believe that neither pep8 nor flake8 should be in the superbuild.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jun 24, 2015

@dlonie until we switch to system python, we need to have pep8 and flake8. Once we fully switch to system python, we can turn it OFF. Although, I think it would be a core package for developers (it should be OFF by default and ON only when we enable testing).

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 24, 2015

That's not correct. pep8 and flake8 just process files -- they don't need to share the same python build as UVCDAT to parse the files or link to anything in UVCDAT. They could easily be used from a system installation.

@durack1
Copy link
Member

@durack1 durack1 commented Jun 24, 2015

@aashish24 @dlonie @doutriaux1 wouldn't it make sense to add another level to the build options, so rather than just having lean, default, all would another testing option be useful, which includes everything from all plus all the test suite dependencies (along with the tests themselves)?

It would be great if the buildbot and travis tests were bumped up to include all the packages in the all build anyway - it seems a bit silly to be running these test systems across a sub-build if they're available to run for the all or (my proposed) testing cases..

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jun 26, 2015

@durack1 we cannot build ALL on travis. That will take too long and will be shut-down by travis. all option should enable testing as well. I think we may need 4 options like:

lean, default, mega, all

@durack1
Copy link
Member

@durack1 durack1 commented Jun 26, 2015

@aashish24 @doutriaux1 in theory when @chaosphere2112 manages to get the LLNL travis account sorted the time limitation will no longer be a problem, so it's close from what I gather.. I really do think once this is sorted testing of the complete (rather than mega) or alternatively my testing suggestion would be possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants