yelp / pushmanager Goto Github PK

If there are multiple simultaneous pushes, if one push is certed, merge conflict detection should be rerun.

If this proves to be too taxing on the queue, perhaps add a button to manually trigger a rerun over all existing pickmes (if the master sha has indeed changed).

The push queue is currently at 51 users(!) so the new push announcement was truncated, leaving off most of the link:

{{{
pushhamster
13:36 [a bunch of usernames]: regular push starting! https://pushmanager.yelpcorp.com/p
}}}

A lot of push requests are getting submodule conflicts such as 'not a fast forward' when the push request doesn't bump those identified submodules.

Most likely, the submodule states aren't pristine when requests are tested. In the reset method,
https://github.com/Yelp/pushmanager/blob/master/pushmanager/core/git.py#L78, the HEAD is reset but submodules are not.

This should help, but I'm not sure if it will completely solve the issue.

troscoe@gitaux:/nail/web/pushmanager (master) $ make restart
serviceinit.d/pushmanager stop
StoppingWARNING:root:Can not load configuration from 'config.yaml'.
WARNING:root:Will try loading defaults from 'config.yaml.example'.
Traceback (most recent call last):
  File "pushmanager_api.py", line 40, in <module>
    app = PushManagerAPIApp()
  File "/nail/web/pushmanager/pushmanager/core/application.py", line 31, in __init__
    self.log = open(self.log_file, 'a+')
IOError: [Errno 2] No such file or directory: '/var/log/pushmanager/api.8843.log'
... [OK]
serviceinit.d/pushmanager start
StartingWARNING:root:Can not load configuration from 'config.yaml'.
WARNING:root:Will try loading defaults from 'config.yaml.example'.
Traceback (most recent call last):
  File "pushmanager_api.py", line 40, in <module>
    app = PushManagerAPIApp()
  File "/nail/web/pushmanager/pushmanager/core/application.py", line 31, in __init__
    self.log = open(self.log_file, 'a+')
IOError: [Errno 2] No such file or directory: '/var/log/pushmanager/api.8843.log'
... [OK]

The restart appears to work ok regardless.

If you first write:

Some commit message

Some extra line
Another extra line

Then edit it again, and you'll get:

Some commit message

Some extra line

Another extra line

Every time you resubmit, the number of blank lines grows ~exponentially.

This is currently what application does and causes background services to start as root.

                self.start_services()
                pid.write(self.pid_file, append=True)

                # Drop privileges
                uid = pwd.getpwnam(Settings.get("username", "www-data"))[2]
                os.setuid(uid)

                tornado.ioloop.IOLoop.instance().start()

In pushmanager/templates/create_request_bookmarklet.js, bug numbers from reviewboard are parsed as:

 var ticketNumberToURL = function(bug) {
        return 'https://' + Settings['trac']['servername'] + '/ticket/' + bug.match(/\d+/)[0];
    };

This should either be more general to handle multiple types of issue ids or point to a separate service that can redirect to the correct place. Internally, we have separate redirect service.

In servlets/commentrequest:

        'comments': SA.func.concat(
            db.push_requests.c.comments,
            '\n\n---\n\nComment from %s:\n\n' % self.current_user,
            comment,

SA.func.concat results in concat() which sqlite3 does not support. This should be made compatible ala #22

If a branch is rebased, the next time merge conflict detection is run on the request, the git fetch will fail to fetch due to non-fastforward.

The fetch should include a + in the refspec. See http://git-scm.com/docs/git-fetch and the second EXAMPLES.

Currently you have to be the owner of an active push

If a pull request ("B") contains another pull request ("A"), then merge ordering matters. Because "B"'s branch contains "A"'s branch, if pull request "B" is merged first, then pull request "A" will result in a noop on merge and should be an error in some way.

The merge conflict detection should be able to recognize this scenario and error out.

These tags are special cases in the push workflow, but this is no longer the case. The tags should be deprecated. For now it should be enough with only removing them from the tag suggestion list shown in the new request form.

SHAs were getting set very slowly since they could be stuck behind conflict checking jobs in the GitQueue. This is caused problems where two verify jobs end up both running after an edit, which would raise a GitError, as well as the issue of external resources reliant on pickme SHA not being usable.

super annoying, when i want to communicate to pushees, to have to either X out of the command window (though i haven't canceled the operation) or say it's complete (when it isn't). let me have the msg buttons!

Every new pickme appears to have this issue when checking conflicts between pickmes. There must be an ordering problem. All remotes should be added and fetched before any merge attempt happens.

Now that we've removed the l10n & l10n-only tags in #59 , we should clean out the special "... && localizables_push_website.py" behavior, styling, etc.

When failing a connection to /pushes after successfully logging in, it appears pushmanager attempted to call finish() twice:

HTTPRequest(protocol='https', host='myhost', method='GET', uri='/pushes', version='HTTP/1.1', remote_ip='myotherhost, body='', headers={'Accept-Language': 'en-US,en;q=0.$
Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/web.py", line 1021, in _stack_context_handle_exception
    raise_exc_info((type, value, traceback))
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/stack_context.py", line 259, in _nested
    yield vars
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/stack_context.py", line 229, in wrapped
    callback(*args, **kwargs)
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/gen.py", line 383, in inner
    self.set_result(key, result)
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/gen.py", line 315, in set_result
    self.run()
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/gen.py", line 345, in run
    yielded = self.gen.send(next)
  File "/myuser/pushmanager/servlets/pushes.py", line 23, in get
    self.finish()
  File "/usr/local/lib/python2.6/dist-packages/tornado-2.4.1-py2.6.egg/tornado/web.py", line 686, in finish
    raise RuntimeError("finish() called twice.  May be caused "
RuntimeError: finish() called twice.  May be caused by using async operations without the @asynchronous decorator.
2013-06-05 16:07:34,440 [8283|MainThread] Cannot send error response after headers written

Steps to reproduce:

1. Install pushmanager, tornado 2.x, and other dependencies.
2. python pushmanager_main.py start.
3. Attempt to log in. Immediately receive 500 error.
4. This traceback will appear in the specified log file for pushmanager. The python processes themselves will continue to run.

myhost, myotherhost, and myuser stand in for my installation IP, my remote IP address, and my installation user, respectively.

pushmanager has the capability to generate links to itself, but it uses the server and port it has bound to in order to generate those links. When pushmanager is run behind a proxy, the port it binds to and the port that users use to access it (i.e. the port the proxy binds to), are not the same.

When I've dealt with this issue in other proxied webapps, I have added configuration knobs that allow me to inform the app "bind to this host and port, but generate links for this host and port". Typically I would then set the webapp to bind to "127.0.0.1:" when run in production.

When a pickme is de-pickmed, other pickmes in the push should be re-queued so that if they conflicted with the now-removed pickme their tags are cleared.

However, in real use cases, re-queueing every pickme is not viable in terms of performance. Depickme-ing a request should only requeue other requests in the push that are marked as conflicting with other pickmes.

Our new workflow has no pushplans. We no longer need the pushplan checklist.

Our pushplan workflow has changed and the checklist should be updated to reflect this.

Our post-certify reminders should all be pre-certify now.

should be simple enough to check whether the sha is already contained in master before checking conflicting / non-ff

Both servlets AddRequest and DiscardRequest send duplicate emails to the request user. These two are different from the other *Request servlets because they can accept multiple request ids.

The final select query:

    request_query = db.push_requests.select().where(
        db.push_requests.c.id.in_(self.request_ids))

somehow duplicates the 1st request in the list. I first discovered this while working on #20 but discovered it in master as well.

When branch names are submitted to pushmanager, it does not strip trailing whitespace, which can lead to failures to parse revisions and fetch the branches.