Merge lp:~tonyyarusso/update-notifier/bug-1167621 into lp:update-notifier

Proposed by Tony Yarusso on 2013-04-11
Status: Work in progress
Proposed branch: lp:~tonyyarusso/update-notifier/bug-1167621
Merge into: lp:update-notifier
Diff against target: 146 lines (+65/-41) 1 file modified
To merge this branch: bzr merge lp:~tonyyarusso/update-notifier/bug-1167621
Reviewer Review Type Date Requested Status
Martin Pitt 2013-05-21 Needs Fixing on 2013-05-21
Ubuntu Core Development Team 2013-04-11 Pending
Review via email: mp+158500@code.launchpad.net

Description of the Change

Make apt_check.py more suitable for calling as a module from another script (LP: #1167621)

To post a comment you must log in.
Iain Lane (laney) wrote :

Hey pitti, looks like this is up your street - fancy reviewing? :-)

(if not, let me know and I'll try to reason it out myself)

Martin Pitt (pitti) wrote :

8 -def write_package_names(outstream, cache, depcache):
9 +def write_package_names(cache, depcache, messages, standalone, delim="\n"):
10 " write out package names that change to outstream "

Can you please adjust the docstrings accordingly? Also, you should rename the methods, as they don't write anything any more. Perhaps get_package_names()? Also, it seems this function doesn't actually use the new "standalone" argument?

For write_human_readable_summary, please also document the two new arguments in the docstring. I think it's an unusual style to pass in "messages" and have the function return that plus some extra text; instead, concatenate the result of the two functions in the caller.

64 + "--delimeter",

It's "delimiter" (same typo again further down).

Thanks!

review: Needs Fixing
Sebastien Bacher (seb128) wrote :

(setting to "work in progress" so it drops from the sponsoring queue while being worked, please set it back to "needs review" again once you addressed the review comments)

Unmerged revisions

790. By Tony Yarusso <email address hidden> on 2013-04-11

Make apt_check.py more suitable for calling as a module from another script (LP: #1167621)

Preview Diff

1=== modified file 'data/apt_check.py'
2--- data/apt_check.py 2012-10-16 13:45:38 +0000
3+++ data/apt_check.py 2013-04-11 21:50:27 +0000
4@@ -48,25 +48,57 @@
5 return True
6 return False
7
8-def write_package_names(outstream, cache, depcache):
9+def write_package_names(cache, depcache, messages, standalone, delim="\n"):
10 " write out package names that change to outstream "
11 pkgs = filter(lambda pkg:
12 depcache.marked_install(pkg) or depcache.marked_upgrade(pkg),
13 cache.packages)
14- outstream.write("\n".join(map(lambda p: p.name, pkgs)))
15+ messages += delim.join(map(lambda p: p.name, pkgs)) + "\n"
16+ return messages
17
18-def write_human_readable_summary(outstream, upgrades, security_updates):
19+def write_human_readable_summary(upgrades, security_updates, messages, standalone):
20 " write out human summary summary to outstream "
21- outstream.write(gettext.dngettext("update-notifier",
22- "%i package can be updated.",
23- "%i packages can be updated.",
24- upgrades) % upgrades)
25- outstream.write("\n")
26- outstream.write(gettext.dngettext("update-notifier",
27- "%i update is a security update.",
28- "%i updates are security updates.",
29- security_updates) % security_updates)
30- outstream.write("\n")
31+ messages += gettext.dngettext("update-notifier",
32+ "%i package can be updated.",
33+ "%i packages can be updated.",
34+ upgrades) % upgrades
35+ if standalone:
36+ messages += "\n"
37+ else:
38+ messages += " "
39+ messages += gettext.dngettext("update-notifier",
40+ "%i update is a security update.",
41+ "%i updates are security updates.",
42+ security_updates) % security_updates
43+ messages += "\n"
44+ return messages
45+
46+def get_options(args):
47+ parser = OptionParser()
48+ parser.add_option("-p",
49+ "--package-names",
50+ action="store_true",
51+ dest="show_package_names",
52+ help=_("Show the packages that are going to be installed/upgraded"))
53+ parser.add_option("",
54+ "--human-readable",
55+ action="store_true",
56+ dest="readable_output",
57+ help=_("Show human readable output on stdout"))
58+ parser.add_option("",
59+ "--security-updates-unattended",
60+ action="store_true",
61+ help=_("Return the time in days when security updates "
62+ "are installed unattended (0 means disabled)"))
63+ parser.add_option("-d",
64+ "--delimeter",
65+ action="store",
66+ type="string",
67+ dest="pkg_delim",
68+ help=_("Set delimeter to use between package names when used "
69+ "with the -p option. Default is a newline."))
70+ return parser.parse_args(args)
71+
72 def init():
73 " init the system, be nice "
74 # FIXME: do a ionice here too?
75@@ -78,7 +110,9 @@
76 apt_pkg.config.set("Dir::Cache::pkgcache","")
77 apt_pkg.config.set("Dir::Cache::srcpkgcache","")
78
79-def run(options=None):
80+def run(options=None, standalone=False):
81+
82+ messages = ""
83
84 # we are run in "are security updates installed automatically?"
85 # question mode
86@@ -146,18 +180,23 @@
87 break
88
89 # print the number of upgrades
90+
91+ if options and options.readable_output:
92+ messages = write_human_readable_summary(upgrades, security_updates, messages, standalone)
93+
94 if options and options.show_package_names:
95- write_package_names(sys.stderr, cache, depcache)
96- elif options and options.readable_output:
97- write_human_readable_summary(sys.stdout, upgrades, security_updates)
98- else:
99+ if options.pkg_delim:
100+ messages = write_package_names(cache, depcache, messages, standalone, options.pkg_delim)
101+ else:
102+ messages = write_package_names(cache, depcache, messages, standalone)
103+
104+ if (not options or (options and not (options.readable_output or options.show_package_names))) and standalone:
105 # print the number of regular upgrades and the number of
106 # security upgrades
107- sys.stderr.write("%s;%s" % (upgrades,security_updates))
108-
109- # return the number of upgrades (if its used as a module)
110- return(upgrades,security_updates)
111-
112+ messages = str(upgrades) + ";" + str(security_updates)
113+
114+ # return the number of upgrades
115+ return (upgrades, security_updates, messages)
116
117 if __name__ == "__main__":
118 # setup a exception handler to make sure that uncaught stuff goes
119@@ -171,24 +210,9 @@
120 gettext.textdomain(APP)
121
122 # check arguments
123- parser = OptionParser()
124- parser.add_option("-p",
125- "--package-names",
126- action="store_true",
127- dest="show_package_names",
128- help=_("Show the packages that are going to be installed/upgraded"))
129- parser.add_option("",
130- "--human-readable",
131- action="store_true",
132- dest="readable_output",
133- help=_("Show human readable output on stdout"))
134- parser.add_option("",
135- "--security-updates-unattended",
136- action="store_true",
137- help=_("Return the time in days when security updates "
138- "are installed unattended (0 means disabled)"))
139- (options, args) = parser.parse_args()
140+ (options, args) = get_options(sys.argv[1:])
141
142 # run it
143 init()
144- run(options)
145+ result = run(options, True)[2]
146+ sys.stdout.write(result)

Subscribers

People subscribed via source and target branches

to all changes: