[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnewsense-dev] Debderive

From: Sam Geeraerts
Subject: Re: [Gnewsense-dev] Debderive
Date: Mon, 20 Feb 2012 23:16:35 +0100
User-agent: Thunderbird (X11/20101029)

Stayvoid wrote:
I wrote another patch:

@@ -477,9 +477,17 @@

 if __name__ == "__main__":
-    # Check for reprepro
-    if not os.path.exists('/usr/bin/reprepro'):
-        raise IOError, 'Missing dependency: install reprepro and try again.'
+    try:
+        open('/usr/bin/reprepro')
+    except IOError:
+        sys.stderr.write('Missing dependency: install reprepro and
try again.\n')
+#        raise


+    try:
+        open('/etc/debderiver/debderiver.yaml')
+    except IOError:
+        sys.stderr.write('Copy `debderiver.yaml\' to
`/etc/debderiver\' and edit it to suit your needs.\n')
+#        raise

     CONF_DIR_PATH = '/etc/debderiver'

You should take advantage of CONF_DIR_PATH instead of hard coding the same string multiple times.

Which one should I use: "sys.exit(1)" or "raise"? I've read some
discussions, but it's unclear to me.
I'd used "raise" because it provides more information. I'm not sure
that it's necessary to stop the script this way. That's why I
commented it out.
Maybe we should uncomment it if those checks are "critical."

I understand why you'd want to use raise. However, Python's default exception handler (i.e. printing the stack) is an automated best effort approach that exposes the innards of the program. A user is usually not interested in that and just wants a short and clean error message she can act on. Therefore, a message + sys.exit is the better solution.

The script can't run without reprepro or debderiver.yaml, so they're indeed critical.

I used "sys.stderr.write" because (AFAIK) it's compatible with the
third version of Python. Maybe we should replace every "print" with it
("print" statements are only used for the error handling). What do you

Python 2.x is still the default in Parkes, so I've been neglecting taking 3.x into account. I welcome a patch for your idea.

What is the name of the program: "debderive" or "debderiver"?
You use different names for the directory and for the script.
Is there a reason for this or it's just a typo?

I thought to name the tool "Debderiver" (as in: it derives a distro from Debian (or Debian based distro)) and make the command name an imperative. If that doesn't make sense, we can change it. I'm not particularly attached to that choice (or the name in general for that matter).

I'm not a python buff, but isn't The Done Thing to use try blah:, except
What about this?

except IOError:
  print 'Copy `\' to `/usr/local/bin\'\n'

I've looked through the docs and this one looks right. But it doesn't
check the actual file.
And I don't know how to use "if" and "os.path.abspath" with "try."

I'm not sure which file you mean and what you want to do with abspath, but not every check needs to be implemented with an exception. Exceptions are just an elegant way of handling function calls. If a plain if statement is enough, just use that. E.g. check for "os.path.isdir".

reply via email to

[Prev in Thread] Current Thread [Next in Thread]