562026 - hg poller ignores first push to repo

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P5)

Description

[Axel Hecht [:Pike]]

BaseHgPoller.processData throws away the changes it gets if there's no self.lastChangeset.

I guess we could do a 

elif len(change_list) > 0:
  self.lastChangeset = "0" * 12

Comment 1

[Axel Hecht [:Pike]]

grrr, http://hg.mozilla.org/releases/l10n-mozilla-1.9.2/mai/pushlog?fromchange=000000000000 doesn't show what http://hg.mozilla.org/releases/l10n-mozilla-1.9.2/mai/pushlog shows.

Ted, should the pushlog feed support that?

Comment 2

[(not currently active) Ted Mielczarek]

"fromchange" is non-inclusive, and it needs to stay that way so we can use it for regression range linking.

Comment 3

[Chris AtLee [:catlee]]

What about if self.lastChangeset is None, the poller always returns the most recent changeset?

Comment 4

[Axel Hecht [:Pike]]

lastChangeset is None on first run, on purpose. We could also add another flag to denote "first run", I thought that using 12*'0' would do that.

Might be easier to keep that flag inside the poller instead of "fixing" the pushlog api. Though I like the idea that you can get the first push programmatically there either way.

Comment 5

[Chris AtLee [:catlee]]

(In reply to comment #4)
> lastChangeset is None on first run, on purpose. We could also add another flag
> to denote "first run", I thought that using 12*'0' would do that.
> 
> Might be easier to keep that flag inside the poller instead of "fixing" the
> pushlog api. Though I like the idea that you can get the first push
> programmatically there either way.

Oh, sorry, I missed the part where this is about missing the first push to the repo itself!

Comment 6

[Chris AtLee [:catlee]]

Attachment: Submit first pushes

Think something like this could work?

I'm also wondering about the case where a repository is reset.  pushlog will generate an error, so we should probably catch that and reset our state.

Comment 7

[Chris AtLee [:catlee]]

Comment on attachment 453218 [details] [diff] [review]
Submit first pushes

whoops, wrong cc

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 453218 [details] [diff] [review]
Submit first pushes

This should work, I'd revert the logic of firstRun, though, and make that emptyRepo, starting with false. It's probably more descriptive, and closer to what you're testing for, thus making the code easier to understand.

Comment 9

[Chris AtLee [:catlee]]

Attachment: Submit first pushes, and handle reset repositories

So this handles the case where when the poller first starts up the repository is empty.  It tracks this via the emptyRepo variable.

I also added some error handling where if hg reports that it can't find the revision we're asking for, the poller will reset itself and assume the repository has been reset.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 465798 [details] [diff] [review]
Submit first pushes, and handle reset repositories


>+        # If we have a lastChangeset we're comparing against, we've been
>+        # running for a while and so any changes returned here are new.
>+
>+        # If the repository was previously empty (indicated by emptyRepo=True),
>+        # we also want to pay attention to all these pushes.
>+

Is there supposed to be code beneath these comments? It's fine if there isn't, just making sure.


The "emptyRepo" variable name could be the source of confusion -- most cases where this is True will have repository with a bunch of changes but an empty pushlog. I can't think of a better name, though, so r=bhearsum.

Comment 11

[Chris AtLee [:catlee]]

(In reply to comment #10)
> Comment on attachment 465798 [details] [diff] [review]
> Submit first pushes, and handle reset repositories
> 
> 
> >+        # If we have a lastChangeset we're comparing against, we've been
> >+        # running for a while and so any changes returned here are new.
> >+
> >+        # If the repository was previously empty (indicated by emptyRepo=True),
> >+        # we also want to pay attention to all these pushes.
> >+
> 
> Is there supposed to be code beneath these comments? It's fine if there isn't,
> just making sure.

Just trying to make the comments clearer by splitting it up into paragraphs for the different cases.

Comment 12

[Axel Hecht [:Pike]]

Comment on attachment 465798 [details] [diff] [review]
Submit first pushes, and handle reset repositories

Looks good to me, too.

Comment 13

[Chris AtLee [:catlee]]

Comment on attachment 465798 [details] [diff] [review]
Submit first pushes, and handle reset repositories

changeset:   881:bbd53f540b22

Comment 14

[Chris AtLee [:catlee]]

Comment on attachment 465798 [details] [diff] [review]
Submit first pushes, and handle reset repositories

lost the push race...checked in for realz as
changeset:   882:432f46c97889

Comment 15

[Chris AtLee [:catlee]]

Going to call this one fixed.  If it's not working next time a repo is created or reset (e.g. twig repos), then re-open.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

added this new info to https://wiki.mozilla.org/DisposableProjectBranches#What_is_a_disposable_project_branch.3F to let devs know to come here if there are future problems.