Wpt: Simplify maintenance of epochs/* branches

[foolip]

Background: The epochs/* branches in this repository are used to synchronize runs across Azure Pipelines, Taskcluster, Buildbot, and potentially other CI systems to produce results for the same commit. Concretely, the epochs/daily branch triggers revision-aligned stable runs, and the epochs/six_hourly branch is what triggers Azure Pipelines 4 times a day.

The setup for these is more complex than it needs to be:

• wpt.fyi has an in-memory checkout of wpt to power https://wpt.fyi/api/revisions/latest
• wpt-pr-bot polls this and pushes branches in epochs.js

This requires wpt-pr-bot to be always running, which it needs to for other reasons, but I'm hopeful that will change. There are glitches due to timing, see https://github.com/web-platform-tests/wpt-pr-bot/issues/74.

The setup I propose:

• A ./wpt rev-list (à la git rev-list) to list tagged revisions by a configurable interval.
• A GitHub Actions hourly cron job to run ./wpt rev-list and push branches based on the output.

./wpt rev-list would also be useful for generating a list of commits for which we expect results on wpt.fyi, which can help with monitoring.

The net result would be that all of the logic for this is contained in the wpt repo itself, and hopefully then less work to maintain.

@web-platform-tests/wpt-core-team do any of you feel this warrants an RFC.

@clopez and/or @psaavedra may work on this.

[foolip]

@lukebjerring @Hexcles thoughts on the impact of this on wpt.fyi? Continuing to provide /api/revisions/latest would be fine, but we could instrument it and remove it if usage drops to something very low.

[lukebjerring]

I would prefer that wpt.fyi not be a source of Truth for the epochs, so I'm all for it.

[psaavedra]

To understand well enough what is your idea, @foolip, I have two questions:

• What you mean for list tagged revisions . Tagged revisions of ... ?
• What means to push branches in this context?. Commit and push in the _epochs/* branches_, Run tests based on the WPT revisions listed by the ./wpt rev-li command?

[foolip]

@psaavedra when a PR is merged into wpt's master branch there's a script that runs to tag master with a merge_pr_* tag. Those are the tagged revisions, effectively the commits that master had pointed to at some time. This matters because it's possible to merge multiple commits in a PR but the intermediate ones shouldn't be candidates for full runs.

To push branches is simply to move an epochs/* branch to some new commit and push that to the main repo. This can be done on the command line or using the GitHub API.

[psaavedra]

Implemented wpt rev-list: List tagged revisions (aka merge_pr_* tags ) by a configurable interval which works like this:

$ ./wpt rev-list --max-count 5 --until="yesterday"
b0d2aa5daca004ba2085242731373f3ffe92b017
7d2ac66cdffe7c308a1c9a2324c3f5534266df5e
da8f4e657b845b4f6b828adc2e67a63fcfd840ea
beca8464a59440d44a41edbba1989e8d6dc2a38e
2612d6db427a6cf148ade7bd2ceff3444f165877

(aka git rev-list --tags=merge_pr_* --no-walk --max-count 5 --until yesterday)

$ ./wpt rev-list --help
usage: wpt rev-list [-h] [--max-age MAX_AGE] [--skip SKIP] [--since SINCE]
                    [--min_age MIN_AGE] [--max-count MAX_COUNT]
                    [--until UNTIL]

optional arguments:
  -h, --help            show this help message and exit
  --max-age MAX_AGE     Limit the commits output to specified max time range
  --skip SKIP           Skip number commits before starting to show the commit
                        output
  --since SINCE         Show commits more recent than a specific date. For
                        example: 10 week ago
  --min_age MIN_AGE     Limit the commits output to specified min time range
  --max-count MAX_COUNT
                        Limit the number of commits to output
  --until UNTIL         Show commits older than a specific date. For example:
                        yesterday

[psaavedra]

• A GitHub Actions hourly cron job to run ./wpt rev-list and push branches based on the output.

pushes each hash listed by ./wpt rev-list in:

• upstream/epochs/daily
• upstream/epochs/six_hourly
• upstream/epochs/twelve_hourly
• upstream/epochs/weekly

is this OK?

[foolip]

@psaavedra looks great! In order to filter the revisions to the ones matching a certain "epoch length" like daily, weekly or every 6 hours, what extra argument do you think is needed?

[psaavedra]

./wpt rev-list --since="2019-10-02 UTC" --until="2019-10-05 UTC"  

; in ISO like format

or

./wpt rev-list --min-age=1579542400 --min-age=1869542400 

; in timestamp format

[foolip]

I think that would allow listning the latest revision for each epoch, but not multiple revisions, right?

To make it very concrete, the lists of revisions in https://wpt.fyi/api/revisions/list?num_revisions=10 should be possible to reproduce using this command. For all except weekly I think giving an epoch length would suffice. For weekly the cutoff time should be on UTC midnight between Sunday and Monday, so an offset might be needed as well, as I think otherwise you get midnight between Saturday and Sunday. (Our calendar happens to be such.)

[psaavedra]

On Sat, Oct 5, 2019, 18:00 Philip Jägenstedt [e-mail]
wrote:

I think that would allow listning the latest revision for each epoch, but
not multiple revisions, right?

The command returns a reverse sorted list of revision for a given min/max
timestamp interval. Not only the lastest revision

To make it very concrete, the lists of revisions in

https://wpt.fyi/api/revisions/list?num_revisions=10 should be possible to
reproduce using this command. For

You mean, something like this:

./wpt rev-list --weekly/--daily/...
sha1
sha1
sha1
...

?

alll except weekly I think giving an epoch length would suffice. For
weekly the cutoff time should be on UTC midnight between Sunday and Monday,
so an offset might be needed as well, as I think otherwise you get midnight
between Saturday and Sunday. (Our calendar happens to be such.)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/web-platform-tests/wpt/issues/19322?email_source=notifications&email_token=AAC4ABSX3II5M7LRDO3X6T3QNC22FA5CNFSM4I2Y6JVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANVOLI#issuecomment-538662701,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC4ABVXZU3SK2LGXL6VSKDQNC22FANCNFSM4I2Y6JVA
.

[foolip]

Right, something like --daily or --epoch-size=86400, that matches the list of commits that https://wpt.fyi/api/revisions/list?num_revisions=10 currently returns, for one epoch at a time.

The important epochs that we currently actually use are weekly, daily and six_hourly. Whether the epochs are given as a string (prefined) or numerical isn't super important I think.

[psaavedra]

On Sat, Oct 5, 2019, 19:06 Philip Jägenstedt [e-mail]
wrote:

Right, something like --daily or --epoch-size=86400, that matches the
list of commits that https://wpt.fyi/api/revisions/list?num_revisions=10
currently returns, for one epoch at a time.

The important epochs that we currently actually use are weekly, daily and
six_hourly. Whether the epochs are given as a string (prefined) or
numerical isn't super important I think.

Aye. I see.

Just a question. Sorry coz this question is due to I am not so used with
the workflow of the integration infrastructure.

Those --daily/--epoch-size args must to be aligned with:

• the on going day/week. For example: command launched 2019-10-9 12:17:45
  UTC then the --daily means tagged revisions for day: 2019-10-9
• the current time ba 65thsed when the command is executed
• since a concrete revision

?

—

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/web-platform-tests/wpt/issues/19322?email_source=notifications&email_token=AAC4ABTML4AQEBRNHX6Y3C3QNDCRXA5CNFSM4I2Y6JVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANW6HI#issuecomment-538668829,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC4ABQC22YGOZKPCEYC2DLQNDCRXANCNFSM4I2Y6JVA
.

[foolip]

Perhaps an ASCII diagram in the style of https://git-scm.com/docs/git-log will help:

  Fri   Sat   Sun   Mon   Tue   Wed   Thu   Fri   Sat
   |     |     |     |     |     |     |     |     |  
-A---B-C---DEF---G---H--I-----------J-----K-L----M--N--
                                                      ^
                                                     now 

A through N represents (tagged) commits on the master branch spaced out over time. The master branch currently points to N. To list the daily revisions we list the first revisions before each vertical line, which are midnight UTC. Those are the commits that master pointed to at those points in time. In this example, assuming "now" is Saturday evening, the daily revisions are A, C, F, H, I, J, L and M. N is notably not included, and I was a commit master pointed to two midnights in a row.

All other epochs are analogous, as they're evenly spaced marks on a timeline.

To place the candidate commits on a timeline, we use the committer time (not author time) and can assume the commit graph order matches the commit time order. (It's possible to create a repo where this isn't true, but it holds for wpt.)

[foolip]

There are multiple ways one could implement this, but the approach taken in wpt.fyi is to check for each pair of candidate revisions if an UTC midnight (or other epoch boundary) is between the two revisions.

[foolip]

@psaavedra you also asked about "since a concrete revision" and I think using the currently checked out revision is probably fine. It would be trivial to pipe through another revision, but I don't see a need.

[psaavedra]

I just rewrote the command. Now, it looks like this:

$ ./wpt rev-list --num-revisions 5 --epoch-step 604800
c2eb1ace567d212e6faf0fd0a0f3ba97e46c0af7
d3a712a2efa2e0eb4c00aee22aef878e1cb845ac
b52533c5a60cbd0902dc19d325ab1eb594b59138
305ef9380c25d2a0262e38499d9e501aa61ee201
7561b155e100d0db088c9185457eb611313f6807

$ ./wpt rev-list --num-revisions 5 --epoch-step weekly
c2eb1ace567d212e6faf0fd0a0f3ba97e46c0af7
d3a712a2efa2e0eb4c00aee22aef878e1cb845ac
b52533c5a60cbd0902dc19d325ab1eb594b59138
305ef9380c25d2a0262e38499d9e501aa61ee201
7561b155e100d0db088c9185457eb611313f6807

$ ./wpt rev-list --num-revisions 5 --epoch-step weekly  --log-level DEBUG
DEBUG:root:Log level set to 10
DEBUG:root:list_tagged_revisons: {'epoch_until': 1570359780, 'log_level': 'DEBUG', 'num_revisions': '5', 'epoch_step': 'weekly'}
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1570060800 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1569456000 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568851200 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568246400 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1567641600 --no-walk --no-merges master
c2eb1ace567d212e6faf0fd0a0f3ba97e46c0af7
d3a712a2efa2e0eb4c00aee22aef878e1cb845ac
b52533c5a60cbd0902dc19d325ab1eb594b59138
305ef9380c25d2a0262e38499d9e501aa61ee201
7561b155e100d0db088c9185457eb611313f6807

Basically, the rev-list command now follows this logic:

#! python
import calendar
import time
import subprocess

num_revisions = 10
HOURLY = 3600

epoch_now = calendar.timegm(time.gmtime())
epoch_step = HOURLY
for i in range(0,num_revisions):
    min_age = ( epoch_now / epoch_step - i) * epoch_step
    cmd = "git rev-list --max-count=1 --tags=merge_pr_* --min-age=%s --no-walk master" % min_age
    subprocess.Popen(cmd.split())

... so: the command gets the more recent revision closer to to each epoch step.

The current implementation also can be pointed, not only to now, but also to any other epoch time value using the --epoch-until parameter. For example: --epoch-until now, --epoch-until 1570359600:

$ ./wpt rev-list --num-revisions 5 --epoch-step weekly --epoch-until now  --log-level DEBUG
DEBUG:root:Log level set to 10
DEBUG:root:list_tagged_revisons: {'epoch_until': 'now', 'log_level': 'DEBUG', 'num_revisions': '5', 'epoch_step': 'weekly'}
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1570060800 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1569456000 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568851200 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568246400 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1567641600 --no-walk --no-merges master
c2eb1ace567d212e6faf0fd0a0f3ba97e46c0af7
d3a712a2efa2e0eb4c00aee22aef878e1cb845ac
b52533c5a60cbd0902dc19d325ab1eb594b59138
305ef9380c25d2a0262e38499d9e501aa61ee201
7561b155e100d0db088c9185457eb611313f6807

$ ./wpt rev-list --num-revisions 5 --epoch-step weekly --epoch-until 1570300000  --log-level DEBUG
DEBUG:root:Log level set to 10
DEBUG:root:list_tagged_revisons: {'epoch_until': '1570300000', 'log_level': 'DEBUG', 'num_revisions': '5', 'epoch_step': 'weekly'}
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1570060800 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1569456000 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568851200 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1568246400 --no-walk --no-merges master
DEBUG:root:git rev-list --max-count=1 --tags=merge_pr_* --min-age=1567641600 --no-walk --no-merges master
c2eb1ace567d212e6faf0fd0a0f3ba97e46c0af7
d3a712a2efa2e0eb4c00aee22aef878e1cb845ac
b52533c5a60cbd0902dc19d325ab1eb594b59138
305ef9380c25d2a0262e38499d9e501aa61ee201
7561b155e100d0db088c9185457eb611313f6807

[foolip]

Cool, can you verify that it returns the same most recent 100 commits as the wpt.fyi API for all supported epochs?

I see --no-merges in there and pull requests can still be merged with regular merge commits, so I expect that would miss some commits.

[foolip]

I also expect that the use of epoch_now will lead to the results varying depending on when the command is run, even if no midnight or equivalent epoch divider was passed in between.

[foolip]

@psaavedra also it sounds like your subcommand is far enough a long that you could upload it in a pull request and we can review corner cases there.

[psaavedra]

@psaavedra also it sounds like your subcommand is far enough a long that you could upload it in a pull request and we can review corner cases there.

I already created https://github.com/web-platform-tests/wpt/pull/19536

[psaavedra]

Cool, can you verify that it returns the same most recent 100 commits as the wpt.fyi API for all supported epochs?

verified.

I see --no-merges in there and pull requests can still be merged with regular merge commits, so I expect that would miss some commits.

Removed

[psaavedra]

I also expect that the use of epoch_now will lead to the results varying depending on when the command is run, even if no midnight or equivalent epoch divider was passed in between.

can you provide an example?

[foolip]

@psaavedra I'll go ahead and review https://github.com/web-platform-tests/wpt/pull/19536, testing locally to see if there is an issue with epoch_now and local time elapsing.

Also, we discussed last week about a tagging "race condition" that the overall setup for needs to deal with. The state of the repo right now is a good example of this:

commit 643548a2e0cef9324a67e61534d2105c8e6b4c34 (origin/master, origin/HEAD)
Author: Yifan Luo <[email protected]>
Date:   Fri Oct 4 03:03:04 2019 -0700

    Renaming fetch/metadata web platform tests

    Bug: 1011285

    Change-Id: I8f96a36e7fd0a27dde66671c3c85e298236d20f3
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838173
    Reviewed-by: Mike West <[email protected]>
    Commit-Queue: Yifan Luo <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#702780}

commit afcf0c802fcaf17015752b8f91de716c2ed731a8 (tag: merge_pr_19352)
Author: Hiroshige Hayashizaki <[email protected]>
Date:   Sun Oct 6 03:25:06 2019 -0700

    [WPT/upgrade-insecure-requests] Regenerate files

    Follow-up of
    https://chromium-review.googlesource.com/c/chromium/src/+/1788551.

    - Generate gen/ files (mechanical)
    - Remove old test files
    - Update TestExpectations (manual)
        - Added crbug.com/1001374 lines, as this CL adds test coverage
          for <iframe src="(mixed-content)"> inside <iframe src="">
          that is affected by crbug.com/1001374.

    Bug: 1001422, 1001374, 917554
    Change-Id: I17dae578b18b943c5962222f2e0e2c81e6190aa6
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1788553
    Commit-Queue: Hiroshige Hayashizaki <[email protected]>
    Reviewed-by: Kenichi Ishibashi <[email protected]>
    Reviewed-by: Hiroki Nakagawa <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#703184}

commit 28cbc22e946b0a481435ab299490b810b2ec29c8 (tag: merge_pr_18854)
Author: Hiroshige Hayashizaki <[email protected]>
Date:   Wed Oct 2 19:01:23 2019 -0700

    [WPT/referrer-policy] Add worker subresource tests

    This CL adds tests of subresources sent from worker global scope,
    including XHR/Fetch API inside workers and nested workers.

    The tests are failing on Safari and Firefox, largely because
    of differences of referrer policy inheritance rule
    (which is a spec-level issue).

    Bug: 906850
    Change-Id: Ia385d72918df5d4318c7fc58306bfc614a3bd4b8
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1785499
    Commit-Queue: Hiroshige Hayashizaki <[email protected]>
    Reviewed-by: Hiroki Nakagawa <[email protected]>
    Reviewed-by: Kenichi Ishibashi <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#702280}

Whether to handle this in the ./wpt rev-list command or in the code that uses its output I'm not sure about yet.

[psaavedra]

commit 643548a2e0cef9324a67e61534d2105c8e6b4c34 (origin/master, origin/HEAD)
...
commit afcf0c802fcaf17015752b8f91de716c2ed731a8 (tag: merge_pr_19352)

How this is possible ? aren't the tags and the commits in master pushed at the same time?

I don't see how the rev-list command can distinguish between a commit with a missing tag associated ( like 643548a2e0cef9324a67e61534d2105c8e6b4c34 - tag: merge_pr_19501) and other like 96ff703e400e669c37877c01cad02e3c597776d7 which actually belongs to the (tag: merge_pr_19535)

Maybe .... rev-list must to assume HEAD of master is always a tagged revisions even if the tagged was not propagated yet. ¿?¿? :-/ ???

Edit: ^ this only works if all the commits in master comes from Pull Request. Duno if this is the reality.

[foolip]

aren't the tags and the commits in master pushed at the same time?

No, https://github.com/web-platform-tests/wpt/blob/master/.github/workflows/push-build-release-manifest.yml runs in response to a push to master and will push the tag afterwards, usually quickly but it if downloading an existing manifest fails it could take over 4 minutes, and if anything in the process fails the tag won't be created at all, but could be created by hand some time later.

[foolip]

One way to think about this is that the candidates are all the commits that the master branch has ever pointed to. The merge_pr_* tags are those commits, but the commit that master is currently pointing at is one of those commits as well, even if it's not tagged yet.

A wrinkle remains, however. Many commits might be pushed in succession and the might not be tagged in order. Rather than special-casing the commit that master points to, we could perhaps instead set a deadline, to assume that any tags are created within 10 minutes and accept that otherwise things will go wrong. That would certainly simplify matters.

[psaavedra]

A wrinkle remains, however. Many commits might be pushed in succession and the might not be tagged in order. Rather than special-casing the commit that master points to, we could perhaps instead set a deadline, to assume that any tags are created within 10 minutes and accept that otherwise things will go wrong. That would certainly simplify matters.

So, something like this?:

$ ./wpt rev-list --num-revisions 100 --epoch-step weekly --epoch-threshold 600

where --epoch-threshold 600 establish a safety threshold respect now of 600 seconds.

Then, if now = 1570469449, the result of this command:

$ ./wpt rev-list --num-revisions 100 --epoch-step weekly --epoch-threshold 600

it will be equivalent to:

$ ./wpt rev-list --num-revisions 100 --epoch-step weekly  --epoch-until 1570468849

In other words, the rev-list command will ask for revisions with a commit time not older than now - epoch-threshold.

WDYT?

[foolip]

I've taken a look at how long it's taken recently to tag commits, and it looks like it occasionally takes more than 10 minutes still.

In https://github.com/web-platform-tests/wpt/runs/248373304 the first try timed out after 45 minutes and the second succeeded.

In https://github.com/web-platform-tests/wpt/runs/235662622 it took 24 minutes but succeeded.

I'm not sure yet how best to deal with this variability. 95% did finish in less than 10 minutes but 5% is a bit much to accept incorrect results for.

[psaavedra]

I've taken a look at how long it's taken recently to tag commits, and it looks like it occasionally takes more than 10 minutes still.

In https://github.com/web-platform-tests/wpt/runs/248373304 the first try timed out after 45 minutes and the second succeeded.

In https://github.com/web-platform-tests/wpt/runs/235662622 it took 24 minutes but succeeded.

I'm not sure yet how best to deal with this variability. 95% did finish in less than 10 minutes but 5% is a bit much to accept incorrect results for.

yes, not too much reliable. ideally, we need some kind of semaphore or index file pointing to a valid hash which 1) is already pushed in upstream/master 2) is already tagged and pushed in upstream/master as well. it seems easy but apparently not :-/.

[foolip]

What we perhaps could do is to maintain a much simpler set of refs that would effectively be the reflog of master, which doesn't involve creating tags and would have less ways of failing. Perhaps refs/reflog/master/$n or similar. However, whatever creates those refs could still have a bad day and it's not foolproof. I think only having a commit bot that pushes first the tag and then the master branch would provide the guarantees we want. I don't think it's worth building that, however, let's start by just assuming that tags will be created within some fixed number of minutes, and see how often we get the epochs/* branches pointing at the wrong commits as a result. The only bad thing that happens is that we get an extra run of whatever is triggered by the branch that was updated incorrectly and later corrected.

[foolip]

https://github.com/web-platform-tests/wpt/issues/19380 is also relevant as to why tagging can take so long.

[psaavedra]

What we perhaps could do is to maintain a much simpler set of refs that would effectively be the reflog of master, which doesn't involve creating tags and would have less ways of failing. Perhaps refs/reflog/master/$n or similar. However, whatever creates those refs could still have a bad day and it's not foolproof.

Yeah. I think we will falling into the same trap twice.

I think only having a commit bot that pushes first the tag and then the master branch would provide the guarantees we want. I don't think it's worth building that, however, let's start by just assuming that tags will be created within some fixed number of minutes, and see how often we get the epochs/* branches pointing at the wrong commits as a result. The only bad thing that happens is that we get an extra run of whatever is triggered by the branch that was updated incorrectly and later corrected.

It's OK every time we accept this like a eventually-consistent service :-)

[foolip]

Alright, so let's start with the assumption that tags appears within 10 minutes after push to master, and then when that inevitable results in some incorrect push of an epochs/* branch we can decide how serious that problem is. Maybe fixing https://github.com/web-platform-tests/wpt/issues/19380 will be the best way to go about it.

[psaavedra]

Alright, so let's start with the assumption that tags appears within 10 minutes after push to master, and then when that inevitable results in some incorrect push of an epochs/* branch we can decide how serious that problem is. Maybe fixing #19380 will be the best way to go about it.

Added:

 --epoch-threshold EPOCH_THRESHOLD    Safety threshold respect now (default: 600 seconds).

$ ./wpt rev-list --num-revisions 10 --epoch-step weekly --epoch-until now --epoch-threshold 1000  --log-level 10 --verbose --head upstream/master
...
DEBUG:root:epoch-until adjusted to 1570531534 using the safety epoch-threshold value (1000)
...

[psaavedra]

Alright, so let's start with the assumption that tags appears within 10 minutes after push to master, and then when that inevitable results in some incorrect push of an epochs/* branch we can decide how serious that problem is. Maybe fixing #19380 will be the best way to go about it.

WDYT, should I jump into this?

[foolip]

I've been working on some improvements to manifest_build.py today and https://github.com/web-platform-tests/wpt/pull/19580 is my suggestion for making it faster, but I think we can proceed with the 10 minute grace period. I'll review https://github.com/web-platform-tests/wpt/pull/19536 again now under that assumption.

[foolip]

@psaavedra I've submitted a PR to update just one branch, a new epochs/three_hourly, using ./wpt rev-list, in https://github.com/web-platform-tests/wpt/pull/19873.

That's because I actually want to try using it on Azure Pipelines, and if it works it should be pretty simple to migrate the rest as detailed in this issue.

[Hexcles]

Sorry for being late to the party. #19536 looks great to me, which however makes me wonder why we initially decided to implement this on the server side? I don't see this approach (implementing a subcommand in wpt) was discussed in the original design. Did we simply miss it, or am I forgetting something?

[psaavedra]

@psaavedra I've submitted a PR to update just one branch, a new epochs/three_hourly, using ./wpt rev-list, in #19873.

That's because I actually want to try using it on Azure Pipelines, and if it works it should be pretty simple to migrate the rest as detailed in this issue.

LGTM

[foolip]

@Hexcles although I can't find the original public context, I was able to dig out from my inbox and notes a claim by me that @jgraham had said this somewhere:

If we consider putting the data about available revisions in web-platform-tests git, then we open up possibilities like making this function a wpt command in the web-platform-tests repository, which seems like a considerable simplification compared to having to maintain a web service just for this.

This wasn't captured in the design doc as an alternative considered, but I know that @mdittmer and I discussed it. At the time, the running infrastructure was different, and in particular some part of https://github.com/web-platform-tests/results-collection, probably the master, would have needed a full checkout to determine what else to check out and start running tests.

Aside: It's arguably still a bit unsightly that a full checkout is needed in one job (on GitHub Actions) to determine what commit another job (on Azure Pipelines) should check out. However, this isn't a hard requirement, we could actually use the GitHub API to enumerate all releases and update branches, more along the lines of https://github.com/foolip/wpt-epochs-branch-updater/blob/master/update_branches.js. It would transfer less data, but I think it would be slower and less reliable overall.

[foolip]

Oh, also, at the time we didn't have Azure Pipelines or GitHub Actions to schedule actions at precise times of day, so we would still have needed some service running to perform that job. Before the announcer I had https://github.com/foolip/wpt-epochs-branch-updater running on my machine and that wasn't very robust :)

[Hexcles]

That makes a lot of sense! Thanks, Philip!

[psaavedra]

New PR related to the task defined in this issue: https://github.com/web-platform-tests/wpt/pull/19977

This patch unit tests for the list_tagged_revisons(epoch, max_count)
in revlist. The idea is prevent future regressions due to changes done
in this command.

... based on the previous conversation here https://github.com/web-platform-tests/wpt/pull/19536#issuecomment-543065572

[foolip]

@psaavedra with https://github.com/web-platform-tests/wpt/pull/19984 merged I think all that remains here is removing the code in wpt-pr-bot and monitoring that all branches continue to be updated once that change is deployed.

(I would expect that wpt-pr-bot currently usually beats the epochs workflow, so only by turning it off will we see all branches really modified by the workflow.)

[psaavedra]

@psaavedra with #19984 merged I think all that remains here is removing the code in wpt-pr-bot and monitoring that all branches continue to be updated once that change is deployed.

(I would expect that wpt-pr-bot currently usually beats the epochs workflow, so only by turning it off will we see all branches really modified by the workflow.)

Not sure if I catch you all but what I understood is a PR like this one that I created a moment ago: https://github.com/web-platform-tests/wpt-pr-bot/pull/111. Not sure about the monitoring a part of keep tuned watching how the epoch branches are being updated.

[foolip]

https://github.com/web-platform-tests/wpt-pr-bot/pull/111 looks great!

For monitoring, keeping an eye on https://github.com/web-platform-tests/wpt/actions?query=workflow%3Aepochs in the week following https://github.com/web-platform-tests/wpt-pr-bot/pull/111 being merged should be enough, to make sure it successfully updates all the branches at least once. If it falls apart after that, we'll notice pretty quickly.

@stephenmcgruer FYI in case you think more systematic monitoring of the branch maintenance is warranted. I personally think monitoring that runs matching ./wpt rev-list show up on wpt.fyi should suffice, it's more end-to-end.

[psaavedra]

For monitoring, keeping an eye on https://github.com/web-platform-tests/wpt/actions?query=workflow%3Aepochs in the week following web-platform-tests/wpt-pr-bot#111 being merged should be enough, to make sure it successfully updates all the branches at least once. If it falls apart after that, we'll notice pretty quickly.

For the record: https://github.com/web-platform-tests/wpt-pr-bot/pull/111#issuecomment-549232537 .

[foolip]

@psaavedra I think this issue can be closed now, or do you know of any loose ends that should be tied up?

[psaavedra]

No. I think everything is done right now. Let's close it.

On Mon, Nov 4, 2019, 10:01 Philip Jägenstedt [e-mail]
wrote:

@psaavedra https://github.com/psaavedra I think this issue can be
closed now, or do you know of any loose ends that should be tied up?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/web-platform-tests/wpt/issues/19322?email_source=notifications&email_token=AAC4ABRCVH575BLPPP7G7IDQR7QG7A5CNFSM4I2Y6JVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC6SNDI#issuecomment-549267085,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAC4ABWEPO5BNRK72SYB3W3QR7QG7ANCNFSM4I2Y6JVA
.

[foolip]

Thanks @psaavedra! This all went well as I'd hoped :)