From Q to compiler in less than 30 seconds

Question

(See the newer version here: CodeReview question markdown downloader)

As an adjunct to From new Q to compiler in 30 seconds, I've created a Python script to automatically download the markdown from any question on Code Review and save it to a local file using Unix-style line endings.

For instance, to fetch the markdown for this question, one could write:

python fetchQ.py 124479 fetchquestion.md

I'm interested in a general review including style, error handling or any other thing that could be improved.

fetchQ.py

""" Code Review question fetcher.  Given the number of the question, uses
the StackExchange API version 2.2 to fetch the markdown of the question and
write it to a local file with the name given as the second argument. """
import sys
import urllib
import StringIO
import gzip
import json
import HTMLParser

def make_URL(qnumber):
    return 'https://api.stackexchange.com/2.2/questions/'+str(qnumber)+'/?order=desc&sort=activity&site=codereview&filter=!)5IYc5cM9scVj-ftqnOnMD(3TmXe'

def fetch_compressed_data(url):
    compressed = urllib.urlopen(url).read()
    stream = StringIO.StringIO(compressed)
    data = gzip.GzipFile(fileobj=stream).read()
    return data    

def fetch_question_markdown(qnumber):        
    url = make_URL(qnumber)
    try: 
        data = fetch_compressed_data(url)
    except IOError as (err):
        print "Error: {0}: while fetching data from {1}".format(err, url)
        sys.exit()
    try:
        m = json.loads(data)
    except ValueError as (err):
        print "Error: {0}".format(err)
        sys.exit()
    try:
        body = m['items'][0]['body_markdown']
    except KeyError:
        print "Error: item list was empty; bad question number?"
        sys.exit()
    except IndexError:
        print "Error: response does not contain markdown; bad question number?"
        sys.exit()
    h = HTMLParser.HTMLParser()
    md = h.unescape(body)
    return md

if __name__ == '__main__':
    if len(sys.argv) != 3:
        print('Usage: fetchQ questionnumber mdfilename')
        sys.exit()
    qnumber, qname = sys.argv[1:3]

    md = fetch_question_markdown(qnumber)
    with open(qname, 'wb') as f:
            f.write(md.encode('utf-8').replace('\r\n','\n'))

Note: This code and its companion C++ project are now available in a github repo.

And here I went putting all of my newer questions into github so people could get the code easier. Hope I didn't cause you too much pain with my questions. — pacmaninbw
– pacmaninbw ♦, Commented Jul 2, 2016 at 20:46

zondo · Accepted Answer · 2016-04-01 17:31:20Z

My answer's short because I don't see much wrong with it, but this is what I think:

In fetch_compressed_data(), you define data when all you do with it is return it. You should just do return ... directly instead of adding a useless variable.
When you exit in an except: block, you should print the error messages to stderr and you should use a non-zero exit code.
print("Usage: fetchQ... shouldn't hard-code the name of the program. You should use sys.argv[0] or os.path.splitext(sys.argv[0])[0] if you want to remove the .py.
Just after you made sure that sys.argv has three items, you use sys.argv[1:3]. You can just use sys.argv[1:] instead.
The line within your with block is indented by eight spaces, but PEP 8 says that it should be four.

jacwah · Accepted Answer · 2016-04-04 21:16:50Z

I have a couple of suggestions which are not specifically about the code, but rather about the user experience.

The first thing I did was copy-paste the script to a file, make it executable and tried to run it.

$ chmod +x fetchQ.py
$ ./fetchQ.py 124307 autoproject
./fetchQ.py: line 5: $' Code Review question fetcher.  Given the number of the question, uses\nthe StackExchange API version 2.2 to fetch the markdown of the question and\nwrite it to a local file with the name given as the second argument. ': command not found
... etc ...

As you understand, I just assumed the script had a shebang line and instead of using python, Bash tried to interpret the script itself. I think other users might have the same expectations and that you should therefore begin your script with the line #!/usr/bin/env python.

Now, you might have noticed, I accidentally left out the .md extension for the file name. Because the primary use-case for this program is to set up a file for AutoProject, which requires files to have that extension, I think it should automatically be added if no other extension is provided.

Finally, the other argument to the program is the question id. Instead of making users extract the id themselves from the url, I think the user should have the option to pass the full link directly because just pressing CtrlL; CtrlC is easier than by hand selecting the id portion.

Dan Oberlam · Accepted Answer · 2016-04-04 22:46:57Z

I think you could exit a little more cleanly. One is to just do

sys.exit("Something bad happened")

While the other is to do

print "Something bad happened"
sys.exit(SOMETHING_BAD_ERRCODE)

The first saves a somewhat meaningless line, while the second would help automated tools (maybe you want to script it and do something different depending on why it fails). Additionally, error messages should probably be printed to standard error instead of standard output (I often like to pipe the outputs to different files/streams).

I also think you put a little too much in this block

try:
    body = m['items'][0]['body_markdown']
except KeyError:
    print "Error: item list was empty; bad question number?"
    sys.exit()
except IndexError:
    print "Error: response does not contain markdown; bad question number?"
    sys.exit()

It is unclear to me which part is supposed to raise which error, and I think making it more explicit and putting each step into its own try...except would be easier to understand.

I also don't like how you swallow the error entirely - it might be helpful to actually see the traceback. Maybe enable some -v mode to show full error output?

Additionally, it may be worthwhile to use argparse (or similar) to handle the CLI - this makes it easier to specify the CLA and will also make future enhancements easier, and will also be more readily understandable by people reading your code.

Steven Fontanella · Accepted Answer · 2019-12-15 22:03:49Z

1

When you run into an exception, you could use raise (by itself) instead of sys.exit() to re-raise the exception. This way the user can see the stack trace when there's a failure and any (hypothetical) other consumers of fetch_question_markdown can catch the exception if appropriate.

answered Dec 15, 2019 at 22:03

Steven Fontanella

3732 silver badges8 bronze badges

Add a comment |

Stack Exchange Network

From Q to compiler in less than 30 seconds

fetchQ.py

4 Answers 4

You must log in to answer this question.

Linked

Hot Network Questions

From Q to compiler in less than 30 seconds

fetchQ.py

4 Answers 4

You must log in to answer this question.

Linked

Related

Hot Network Questions