ENH: Remove dependency upon python-apt library by rbuccigrossi · Pull Request #115 · ReproNim/reproman

rbuccigrossi · 2017-09-21T15:52:47Z

The goal is to remove our dependency upon the python-apt library by using apt-cache calls.

Split _create_package into reasonable methods
Parse source_name and source_version (from Source: line if different than package)
Get the install_date for a package (from /var/lib/dpkg/info/package.list
Get the APTSource date from /var/lib/apt/lists/*Release file ("Date:" tag)
More unit testing

codecov-io · 2017-09-21T16:01:32Z

Codecov Report

Merging #115 into master will increase coverage by 0.19%.
The diff coverage is 91.28%.

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage    75.4%   75.59%   +0.19%     
==========================================
  Files         118      118              
  Lines        7915     8065     +150     
==========================================
+ Hits         5968     6097     +129     
- Misses       1947     1968      +21

Impacted Files	Coverage Δ
niceman/interface/retrace.py	`98.33% <100%> (+3.78%)`	⬆️
niceman/distributions/tests/test_debian.py	`95.74% <100%> (+2.52%)`	⬆️
niceman/resource/tests/test_shell.py	`100% <100%> (ø)`	⬆️
niceman/distributions/base.py	`87.8% <100%> (+0.15%)`	⬆️
niceman/support/distributions/debian.py	`94.89% <100%> (-5.11%)`	⬇️
niceman/tests/test_tests_utils.py	`97.46% <100%> (+0.07%)`	⬆️
niceman/support/distributions/tests/test_debian.py	`100% <100%> (+15.38%)`	⬆️
niceman/resource/shell.py	`74.07% <100%> (+0.99%)`	⬆️
niceman/distributions/debian.py	`89.91% <86.2%> (-1.22%)`	⬇️
niceman/tests/utils.py	`77.99% <87.5%> (+0.3%)`	⬆️
... and 4 more

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 2e53e36...d725b9e. Read the comment docs.

…m apt-cache policy pkg

yarikoptic

minor quick comments

yarikoptic · 2017-09-24T17:54:14Z

niceman/support/distributions/debian.py

+    for entry in entries:
+        match = re_pkg.match(entry.strip())
+        if not match:
+            print("FAILED in ", entry)


lgr.warning?

yarikoptic · 2017-09-28T03:39:06Z

niceman/support/distributions/debian.py

+
+    # Split before PGP signature if present
+    if "-----BEGIN PGP SIGNATURE-----" in content:
+        content = content.split("-----BEGIN PGP SIGNATURE-----")[0]


couldn't it be without 'if' statement and just the content = content.split("-----BEGIN PGP SIGNATURE-----")[0]? ;-)

Look at that! That's nice behavior for str.split()

… structrues

…directory)

…to those tracers that can handle them

… respective dpkg info file

rbuccigrossi · 2017-10-12T21:04:20Z

Assigned to Yarik for review and next steps.

yarikoptic

Grr.. I was certain that I have submitted the review yesterday, but found today that comments weren't yet finalized! sorry!

What if we merge it in the current shape and you would address requested changes in a separate followup PR? This way we could resolve quickly conflicts with @chaselgrove changes in #121 before piling up more of them.
In other words , if approve, merge please ;)

yarikoptic · 2017-10-13T03:39:09Z

niceman/distributions/tests/test_debian.py

-from niceman.tests.utils import skip_if


-@skip_if(not apt)


well, we still will need to skip it on non-debian systems. probably entire module (unless it is some mocked up test).
May be, define in external_versions a new checker for cmd:apt-cache (see the one for git) and then skip here if version is "None" (not installed ie).

Done. I added a skip_if_no_apt_cache decorator in 7a6337b.

yarikoptic · 2017-10-13T03:41:17Z

niceman/distributions/debian.py

+                        datetime.utcfromtimestamp(
+                            mktime_tz(parsedate_tz(spec.date)))))
+                except TypeError as _:
+                    lgr.warning("Unexpected date format %s " % spec.date)


should we may be just blow completely for now so we could detect those instead of missing all possible hundreds of warnings ;)

yarikoptic · 2017-10-13T03:42:06Z

niceman/distributions/debian.py

+                except TypeError as _:
+                    lgr.warning("Unexpected date format %s " % spec.date)
+                break
+            except CommandError as _:


shouldn't there at least be a warning or better even exception? otherwise it might be tricky later to find out why we are missing some information

The issue is that we have two different potential file name endings ("Release" and "InRelease"), so we expect a bunch of failures in attempting to read the files. I'll put better documentation in a comment.

yarikoptic · 2017-10-13T03:45:36Z

niceman/distributions/tests/test_debian.py

+
    assert parse('diversion by dash from: /bin/sh') is None




could there be a sledgehummer test which would (after we start throwing exceptions whenever things go not as planned, see above comments) test on some good number of packages (or even all packages) that it could trace it... how long will it take do you think e.g. on your ubuntu? you could probably try to trace /usr/bin/ and /usr/lib/* /usr/lib// for starters ;)
test itself could verify that all apt sources have some date, all packages have version and architecture, and some apt source assigned.

yarikoptic · 2017-10-13T03:46:39Z

niceman/resource/tests/test_shell.py

+@with_tempfile(content="content")
+def test_isdir(script=None):
+    ses = ShellSession()
+    assert not ses.isdir(script)


tempfile is a heavy artillery for this... why not just __FILE__? ;)

yarikoptic · 2017-10-13T03:48:30Z

niceman/tests/utils.py

+                raise AssertionError("Key %s is missing" % key)
+            assert_is_subset_dict_recur(a[key], b[key])
+    else:
+        if not a == b:


just a note -- there is still != in Python ;)

Yes. And, I love the fact that my assertion error used != while I used "not =="

yarikoptic · 2017-10-13T20:26:17Z

Eh, merging Matt's PR introduced some minor conflicts... I will resolve them now

* origin/master: (39 commits) BF: changed travis python apt packages ENH: added exec interface ENH: Added mkdir, get and put interfaces BF: Added back in some commented out tests for SSH ENH: Made code changes recommended by comments. Fixed python 3 issues. DOC: restored nose install; using pytest directly for testing DOC: updated to reflect new testing (not nose) allow downgrades never give up! BF: adjust apt installation for trusty BF: Retry Travis internal fault. ENH: Changed AWS resource to use Ssh package to connect to remote EC2 servers. BF: Skipping SSH test if no network found. (try 2) BF: Skipping SSH test if no network found. BF: removal of Travis build_install_apt BF: reverting Travis distribution back to precise BF: seeing if Travis installs the docker service on Trusty BF: Restored retrace package test. RF: Used six package for NoSectionError exception. ENH: Moved SSH connection session out of execute command methods. ...

rbuccigrossi · 2017-10-13T21:09:44Z

I'm merging to minimize future conflicts, but will resolve suggestions in a future pull request.

Addressing code review comments for debian identification (#115)

yarikoptic and others added 2 commits September 18, 2017 23:52

ENH: try to come up with re-based parsing of apt-cache policy pkg output

fdf6d4f

BF: Fixed print format for Python 3

546494d

rbuccigrossi added enhancement WiP labels Sep 21, 2017

Robert Buccigrossi added 3 commits September 24, 2017 12:42

ENH: Initial parsing of apt-cache policy to get source information

dbac501

ENH: Rewrote get_spec_from_release_file to remove debian dependency

b5cccda

ENH: Improve spec_from_release_file and finish parsing of sources fro…

5744993

…m apt-cache policy pkg

yarikoptic reviewed Sep 28, 2017

View reviewed changes

Robert Buccigrossi added 18 commits September 28, 2017 17:33

ENH: Added assertion test for dictionary subsets

00e370e

ENH: Added parsing of apt-cache show pkg, added sanity checks of test…

b0cfadb

… structrues

WIP ENH: Calling dpkg and apt-cache to populate Debian retrace objects

74a63bb

WIP ENH: Pulling in sources that are used and renaming them

23da77a

ENH: Cleaner naming of APTSources

a6adf76

BF: Do not break if dpkg-query returns multiple packages for a file (…

734b9df

…directory)

ENH: Filter files that are not directories

2ffa9a3

ENH: Define local ShellSession isdir

47b2d42

ENH: Added HANDLES_DIRS to DistibutionTracers, Only pass directories …

e5cb4fc

…to those tracers that can handle them

ENH: Removed unnecessarily empty file list check

321802c

ENH: Parse source_name and source_version from apt-cache show

de5a03c

ENH: Determine package install date from the modification time of the…

e2468ae

… respective dpkg info file

BF: Restore removed upstream_name from DEBPackage providence class

79be855

ENH: Get the date from the release file for a source

9687915

BF: Fixing parse error of apt-cache policy that skipped first source

c6bfdec

ENH: Code cleanup and using session.read() for Release files

0f79028

DOC: Cleaning up comments

0174de5

ENH: Refactored _create_package to simplify code

d5931ad

rbuccigrossi removed the WiP label Oct 12, 2017

rbuccigrossi changed the title ~~WIP ENH: Remove dependency upon python-apt library~~ ENH: Remove dependency upon python-apt library Oct 12, 2017

ENH: Try to build on travis without python-apt

752a15b

rbuccigrossi assigned yarikoptic Oct 12, 2017

yarikoptic mentioned this pull request Oct 13, 2017

NF: immutable packages #121

Merged

yarikoptic reviewed Oct 13, 2017

View reviewed changes

rbuccigrossi merged commit 3fa69df into ReproNim:master Oct 13, 2017

yarikoptic added a commit that referenced this pull request Nov 8, 2017

Merge pull request #131 from rbuccigrossi/enh-remove_python_apt

ccba01c

Addressing code review comments for debian identification (#115)

Conversation

rbuccigrossi commented Sep 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yarikoptic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbuccigrossi commented Oct 12, 2017

Uh oh!

yarikoptic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yarikoptic commented Oct 13, 2017

Uh oh!

rbuccigrossi commented Oct 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rbuccigrossi commented Sep 21, 2017 •

edited

Loading

codecov-io commented Sep 21, 2017 •

edited

Loading