Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29323474: Issue 2896 - Automate uploading of Firefox development builds to AMO (Closed)

Created:
Aug. 12, 2015, 11:45 a.m. by Wladimir Palant
Modified:
Aug. 18, 2015, 9:04 a.m.
Visibility:
Public.

Description

I didn`t clean up the imports to keep the changeset small but I did remove the setupStderr() call.

Patch Set 1 #

Total comments: 12

Patch Set 2 : Cleaned up imports #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -4 lines) Patch
M sitescripts/extensions/bin/createNightlies.py View 1 1 chunk +16 lines, -4 lines 0 comments Download

Messages

Total messages: 6
Wladimir Palant
Aug. 12, 2015, 11:45 a.m. (2015-08-12 11:45:58 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py#newcode394 sitescripts/extensions/bin/createNightlies.py:394: def load_url(url, data=None): Any reason, why you don'T simply ...
Aug. 12, 2015, 1:29 p.m. (2015-08-12 13:29:26 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py#newcode394 sitescripts/extensions/bin/createNightlies.py:394: def load_url(url, data=None): On 2015/08/12 13:29:26, Sebastian Noack wrote: ...
Aug. 12, 2015, 1:34 p.m. (2015-08-12 13:34:10 UTC) #3
Felix Dahlke
LGTM as far as I'm concerned, with a nit. https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py#newcode28 sitescripts/extensions/bin/createNightlies.py:28: ...
Aug. 13, 2015, 12:52 p.m. (2015-08-13 12:52:27 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensions/bin/createNightlies.py#newcode28 sitescripts/extensions/bin/createNightlies.py:28: import sys, os, os.path, subprocess, ConfigParser, json, hashlib On ...
Aug. 13, 2015, 1 p.m. (2015-08-13 13:00:40 UTC) #5
Sebastian Noack
Aug. 18, 2015, 9:04 a.m. (2015-08-18 09:04:13 UTC) #6
Message was sent while issue was closed.
LGTM

https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensi...
File sitescripts/extensions/bin/createNightlies.py (right):

https://codereview.adblockplus.org/29323474/diff/29323475/sitescripts/extensi...
sitescripts/extensions/bin/createNightlies.py:409: class
CSRFParser(HTMLParser.HTMLParser):
On 2015/08/13 12:52:27, Felix Dahlke wrote:
> On 2015/08/12 13:34:10, Wladimir Palant wrote:
> > On 2015/08/12 13:29:26, Sebastian Noack wrote:
> > > Don't we already use minidom somewhere else? That would be simpler to use
> than
> > > implementing a parser.
> > 
> > Does minidom parse HTML? Not that I know.
> 
> I've used BeautifulSoup for this kind of stuff. But as far as I'm concerned,
we
> can leave it as it is, we're just trying to get the CSRF token after all.

Alright, minidom is only for XML, though it also works with some HTML input. But
what you actually would need is lxml which has proper (and quite error-tolerant)
support of HTML. Note that BeautifulSoup is just an obscure wrapper around lxml
(and some other libraries). So if we add a dependency here, I'd rather use lxml
directly. However, I'm also fine with with sticking to HTMLParser, which comes
with the Python corelib, rather than adding a new dependency.

Powered by Google App Engine
This is Rietveld