Devserver: allow SubversionFileSystem to be pinned to a specific rev on construction

Docserver: don't allow updating subversion beyond when the app itself existed.
In other words, if the currently running app is 2-0-1 and a change is pushed to
bring it to 2-0-2, never fetch any new content from then onwards. This will
allow for parallel content + server pushes.

BUG=236600
R=cduvall@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199503

[not at google - send to devlin, 2013-05-10 01:15:09]

Phew! Done! The presubmit checks are a bit hacky for now - there are more I want
to do - and I also want to do the thing where we only freeze subversion for a
push like 2-0-0 -> 2-1-0 not 2-0-1 - but that can come later.

Some of the logic is a bit gnarly. Pick through with a fine comb. I am still
trying to think how to test this on the prod server. Maybe I can use the dev
server actually...

Anyway, in the meantime, here you go.

[not at google - send to devlin, 2013-05-10 05:34:16]

Tried with the dev server by changing the local version of app.yaml to an older
one. It gets old versions! Pretty sweet!

Making a couple of tweaks to the crons to make them actually work locally in a
reasonable amount of time.

[cduvall, 2013-05-10 06:45:19]

Wasn't able to test locally because I'm not at work, but if it works, the code
lgtm.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/app_yaml_helper_test.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/app_yaml_helper_test.py:90: assert
revision is not None
use self.assert* here?

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron_servlet.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:185:
get_branch_for_channel = self._GetBranchForChannel
is this line necessary? does it not like you using self._GetBranchForChannel
directly below?

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron_servlet_test.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet_test.py:70: def
testSafeRevision(self):
nice tests

[方觉(Fang Jue), 2013-05-10 08:49:17]

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
chrome/common/extensions/docs/server2/PRESUBMIT.py:61: No? Are you sure? How
much do you bet? This can't be rolled back...
Different versions have separate object stores, which means it can be rolled
back if no change is made to the templates. Does server only change really need
to bump the middle version?

Since 'THIS WILL CAUSE THE CURRENTLY RUNNING SERVER TO STOP UPDATING', the
middle version shouldn't be changed more often than necessary? I think only
patches with both server changes and template changes need to bump it?

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
File chrome/common/extensions/docs/server2/app_yaml_helper.py (right):

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
chrome/common/extensions/docs/server2/app_yaml_helper.py:55: lhs_parts =
lhs.replace('-', '.').split('.')
split('-')?

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
File chrome/common/extensions/docs/server2/cron.yaml (right):

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
chrome/common/extensions/docs/server2/cron.yaml:5: target: 2-1-0
perhaps also add presubmit warning if the versions here don't match that in
app.yaml?

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
File chrome/common/extensions/docs/server2/cron_servlet.py (right):

https://chromiumcodereview.appspot.com/14247024/diff/28001/chrome/common/exte...
chrome/common/extensions/docs/server2/cron_servlet.py:199: return
server_instance_at_head
I was wondering what will happen if at this point a new commit (with changes to
both server2 and templates) happens to come in. Perhaps it would better to first
determine current head revision, get app.yaml version, and then pin to that head
revision if the server is up-to-date?

[not at google - send to devlin, 2013-05-10 17:49:58]

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/PRESUBMIT.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/PRESUBMIT.py:61: No? Are you sure? How
much do you bet? This can't be rolled back...
On 2013/05/10 08:49:17, 方觉(Fang Jue) wrote:
> Different versions have separate object stores, which means it can be rolled
> back if no change is made to the templates. Does server only change really
need
> to bump the middle version?
> 
> Since 'THIS WILL CAUSE THE CURRENTLY RUNNING SERVER TO STOP UPDATING', the
> middle version shouldn't be changed more often than necessary? I think only
> patches with both server changes and template changes need to bump it?

Yep, this comment is *slightly* misleading. In a follow-up I'm going to
implement not pinning subversion unless one of the first two numbers change,
however, I just need to get this part in for now.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/app_yaml_helper.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/app_yaml_helper.py:55: lhs_parts =
lhs.replace('-', '.').split('.')
On 2013/05/10 08:49:17, 方觉(Fang Jue) wrote:
> split('-')?

I figure somebody might switch to . rather than - at some point. May as well be
safe.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/app_yaml_helper_test.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/app_yaml_helper_test.py:90: assert
revision is not None
On 2013/05/10 06:45:19, cduvall wrote:
> use self.assert* here?

Done.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron.yaml (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron.yaml:5: target: 2-1-0
On 2013/05/10 08:49:17, 方觉(Fang Jue) wrote:
> perhaps also add presubmit warning if the versions here don't match that in
> app.yaml?

That was actually on my TODO list but writing presubmit checks is incredibly
painful.

I've done it though, now, and wow. What a pain. Oh well, done now.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron_servlet.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:185:
get_branch_for_channel = self._GetBranchForChannel
On 2013/05/10 06:45:19, cduvall wrote:
> is this line necessary? does it not like you using self._GetBranchForChannel
> directly below?

Yeah, I either do this or save a reference to CronServlet's self object, since
the self in this scope overrides it. I don't really know what the idiom is here.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet.py:199: return
server_instance_at_head
On 2013/05/10 08:49:17, 方觉(Fang Jue) wrote:
> I was wondering what will happen if at this point a new commit (with changes
to
> both server2 and templates) happens to come in. Perhaps it would better to
first
> determine current head revision, get app.yaml version, and then pin to that
head
> revision if the server is up-to-date?

Yes true. I have a bug filed about this. There's actually already a bug here
that this won't fix, namely that while the cron is updating these files any
instances in the middle of querying stuff (e.g. static resources) will be
temporarily incorrect. That's more insidious - it'll actually happen, whereas
the race condition you describe ... probably won't.

I had a go at fixing it but it's a yak shave to make the paths work properly
when just reading '/'. I'll do it in a follow-up, gotta submit this today.

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
File chrome/common/extensions/docs/server2/cron_servlet_test.py (right):

https://codereview.chromium.org/14247024/diff/28001/chrome/common/extensions/...
chrome/common/extensions/docs/server2/cron_servlet_test.py:70: def
testSafeRevision(self):
On 2013/05/10 06:45:19, cduvall wrote:
> nice tests

:)

[not at google - send to devlin, 2013-05-10 17:50:39]

Committed patchset #12 manually as r199503.