markdrago / banter Goto Github PK

Is there a reason that the Crucible class uses staticmethod everywhere? If you change to a normal method, then a subclass can override these methods. Because they are called like Crucible.get_create_review_payload, instead of self.get_create_review_payload, subclassing to modify won't work.

See traceback below. The underlying problem is a 404 in this case, you probably want to use request.raise_for_status(). As a separate question I'm surprised the url (/rest-service/auth-v1/login) 404s.. banter probably needs handshaking to determine whether or not it is compatible with the remote crucible server

[c12067:~/.virtualenvs/PORTALDEV-24925/src/branches/PORTALDEV-24925/iqh] [PORTALDEV-24925] PORTALDEV-24925 ‡ banter --setup
Please enter the URL for crucible: http://crucible05.example.com
Please enter the crucible project key: project
Please enter your crucible username: user
Enter a comma-separated list of usernames for anyone you'd like to include on reviews by default: user
In order to get an auth token from crucible, you must enter your password once.
Password:
Traceback (most recent call last):
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/bin/banter", line 11, in
sys.exit(main())
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/banter/banter.py", line 16, in main
setup()
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/banter/banter.py", line 67, in setup
set_crucible_token(updated_conf)
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/banter/banter.py", line 74, in set_crucible_token
token = crucible_conn.get_auth_token(conf['crucible']['username'], conf['crucible']['password'])
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/banter/crucible.py", line 14, in get_auth_token
if response.json() == "" or response.json() is None:
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/requests/models.py", line 805, in json
return complexjson.loads(self.text, **kwargs)
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/simplejson/init.py", line 451, in loads
return _default_decoder.decode(s)
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/simplejson/decoder.py", line 406, in decode
obj, end = self.raw_decode(s)
File "/Users/c12067/.virtualenvs/PORTALDEV-24925/lib/python2.7/site-packages/simplejson/decoder.py", line 426, in raw_decode
raise JSONDecodeError("No JSON object could be decoded", s, idx)
simplejson.decoder.JSONDecodeError: No JSON object could be decoded: line 1 column 0 (char 0)

When the patch gets sent up to crucible there's a leading "a" directory on all modified files. If there are new files they end up under a leading "b" directory. I'm pretty sure this is due to the patch header for new files looking like this:

diff --git a/myfile.java b/myfile.java
new file mode 100644
--- /dev/null
+++ b/myfile.java

I've come up with two options to fix this:

1. replace /dev/null with the "a" version of the file mentioned on the +++ line
2. remove all "a"s and "b"s.

Option 2 is preferable if that works out as it will likely make the real paths show up in crucible rather than show any prefixed directory.

My only reservation is that there will now be code in banter which modifies the diff that is being passed through.

Edit: After doing some investigation, it turns out that this package doesn't work when the crucible url uses https instead of http. Is that a crucible limitation or a bug in banter?