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

pytablereader seems to require a newer version of requests #123

Merged
merged 2 commits into from
Oct 18, 2017
Merged

pytablereader seems to require a newer version of requests #123

merged 2 commits into from
Oct 18, 2017

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Oct 17, 2017

When running the version of pshtt in master via domain-scan, I encountered scores of the following error:

Error running ['pshtt', '012.tlsa.good.test.dnsops.gov', '--json', '--user-agent', '"github.com/18f/domain-scan, pshtt.py"', '--timeout', '30', '--preload-cache', './cache/preload-list.json'].
scan_1 | Bad news scanning, sorry!
scan_1 | [090.nihweb5.cit.nih.gov][pshtt]
scan_1 | pshtt 090.nihweb5.cit.nih.gov
scan_1 | Traceback (most recent call last):
scan_1 | File "/usr/local/lib/python3.4/dist-packages/pkg_resources/init.py", line 658, in _build_master
scan_1 | ws.require(requires)
scan_1 | File "/usr/local/lib/python3.4/dist-packages/pkg_resources/init.py", line 972, in require
scan_1 | needed = self.resolve(parse_requirements(requirements))
scan_1 | File "/usr/local/lib/python3.4/dist-packages/pkg_resources/init.py", line 863, in resolve
scan_1 | raise VersionConflict(dist, req).with_context(dependent_req)
scan_1 | pkg_resources.ContextualVersionConflict: (requests 2.14.2 (/usr/local/lib/python3.4/dist-packages), Requirement.parse('requests>=2.18.4'), {'pytablereader'})

I was able to make these errors go away by updating the version of requests in requirements.txt.

@IanLee1521
Copy link
Collaborator

Relates to #121 which adds back testing for Python 3.4.

One thought, which relates a bit to #124 and #111 is, do you know what version of pshtt you're using? Are you using the development version in your local install? Or an older version? Also, do you know the version (or commit) of domain-scan you're running and the command line you're using?

Thanks!

@jsf9k
Copy link
Member Author

jsf9k commented Oct 17, 2017

Thanks for the quick response, @IanLee1521!

I'm pulling the latest from the master branch for both pshtt and domain-scan. I'm running it in a Docker container that is based on ubuntu:14.04.4. (The Dockerfile appears to be a modified version of the domain-scan Dockerfile, and it pulls domain-scan and pshtt from GitHub instead of pip.) After I gather a bunch of domains to be tested into scanme.csv, the command being run is /home/scanner/domain-scan/scan scanme.csv --scan=pshtt --force --debug.

I'm happy to supply more details if you want.

This is to match the change made to the requirements.txt file
@IanLee1521
Copy link
Collaborator

That works for me. I'm fine to merge this, I just checked and I am locally using 2.18.4 myself.

I am a bit unsure why the tests haven't caught this issue in the past though... E.g. in PR #121 I add Python 3.4 support, and requests==2.14.2 is what gets installed along with pytablewriter==0.24.0 so that's a bit strange... Do you have a copy of the Dockerfile you are using that you could share? I'm wondering if there is something in there about the way that pshtt is getting installed.

I did make one change on your branch to make the same version change in the setup.py file to match your proposed change to the requirements file.

@jsf9k
Copy link
Member Author

jsf9k commented Oct 18, 2017

Here is the Dockerfile. I removed two lines that contained the CENSYS API token, but you can see how pshtt is getting installed. Note that I'm currently pulling from my branch so I get the change I committed.

Good catch on the setup.py file!

@IanLee1521 IanLee1521 merged commit 2de867e into cisagov:master Oct 18, 2017
@IanLee1521
Copy link
Collaborator

Thanks again @jsf9k !

@jsf9k jsf9k deleted the bugfix/update_requests_version branch November 8, 2017 21:58
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.

None yet

2 participants