Skip to content

Add removed_intervals to /state #182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add removed_intervals to /state #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a64c48
Add removed intervals to state
niemela Dec 10, 2024
125f8c6
Update changelog
niemela Dec 10, 2024
25b8f8b
Reformat and extend explanation
niemela Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions Contest_API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ absolute timestamps.
containing human-readable time durations, given in a slight modification of
the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) extended time format:
`(-)?(h)*h:mm:ss(.uuu)?`
- Time intervals (type **`INTERVAL`** in the specification) are strings
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the end of the interval is not known yet.

I think that is a case @johnbrvc and @clevengr would like to have, where they 'pause' the contest and so only know the start of the interval and later update it. Or are you saying in that case they should only emit a state update event once the end is also known?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, one of the use cases is something like a fire alarm where you literally hit the 'stop' button and walk out the door. Yes, we could leave the clock running in subsystems and set the end/update the event afterward, but it would be nice for end time to be optional.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec does not allow for open intervals. A removed interval always has a beginning and an end.

In the case you describe, i.e. you know when you want the removed interval to start but not yet when it will end, there are two possible solutions. You could either not set any removed interval yet, or you could set it to some value "far enough" in the future. Either is fine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the spec to allow for open intervals? I'm not in favor or against, but if we are thinking about implementing removed intervals properly now, this is the time to discuss this I'd say.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see Fredrik has already 👎🏼 our comments and maybe we don't. 🙂 I'm just not sure I love forcing one which could end up with either nothing sent ('contest is still running' when clearly it isn't) or far future ('contest will resume in 1865 days') is great either.

Instead of adding a new type (will we need INTERVAL anywhere else?) why not a child object like location: { start: TIME, end: TIME }?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the spec to allow for open intervals? I'm not in favor or against, but if we are thinking about implementing removed intervals properly now, this is the time to discuss this I'd say.

I don't think we do. The simplest argument is that there is always a trivial workaround. Just pick any large enough end and you practically have an open interval anyway.

I'm just not sure I love forcing one which could end up with either nothing sent ('contest is still running' when clearly it isn't)

Well, it is though. If it actually isn't running then it has ended, and there are other messages you should be sending.

or far future ('contest will resume in 1865 days') is great either.

Yeah, I agree that this is a bit ugly, but it's or the most cornery of cases, I think it's fine.

Instead of adding a new type (will we need INTERVAL anywhere else?) why not a child object like location: { start: TIME, end: TIME }?

That's a great point, and in fact that is exactly how I first wrote it. That said, there is a standard ISO format for time intervals in ISO 8601, and we use ISO 8601 formats whenever available elsewhere, so it makes sense to do the same here. I think as I wrote it now is cleaner and better, but I don't feel very strongly about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just not sure I love forcing one which could end up with either nothing sent ('contest is still running' when clearly it isn't)

Well, it is though. If it actually isn't running then it has ended, and there are other messages you should be sending.

I meant more that if all the CCS has is a pause and resume capability, they might decide not to send an event on pause, which would be missing the point of this change. We should set the expectation that it should send an event on pause with a long interval, and make sure someone from PC^2 has reviewed this too b/c I think that's what they have.

Instead of adding a new type (will we need INTERVAL anywhere else?) why not a child object like location: { start: TIME, end: TIME }?

That's a great point, and in fact that is exactly how I first wrote it. That said, there is a standard ISO format for time intervals in ISO 8601, and we use ISO 8601 formats whenever available elsewhere, so it makes sense to do the same here. I think as I wrote it now is cleaner and better, but I don't feel very strongly about that.

I lean slightly the other way that it's not worth adding a global type for this when time would be enough - but I don't feel very strongly about it either. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant more that if all the CCS has is a pause and resume capability, they might decide not to send an event on pause, which would be missing the point of this change.

Sure, but they also could send an event.

We should set the expectation that it should send an event on pause with a long interval,

Do you mean adding a comment to the effect of "If the end of the removed interval is not known, a system should provide a best estimate, or at least some very large (such as 1 day later) value for the end"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Somehow it seemed I added a duplicate comment yesterday, then removed one, and both were gone. Hree goes again.]

I think I'm not too much a fan of open-ended removed intervals, but if we support that, I object to using an end time far in the future to indicate that. I think it's misleading and might lead to problems, for example, if that end time is not that far in the future and it passes without someone noticing. Also, it means that this end time will get modified again, and I think we should try to write the specification to make things as immutable as possible.

Also, in general, I feel like it would be better to just use separate ISO date/time fields for start and end time, because a (well-defined) interval can have various formats:

<start>/<end>
<start>/<duration>
<duration>/<end>

I don't see the benefit of allowing these.

containing human-readable time intervals, given in
[ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) "start and end" format with
a forward slash interval designator.
- Identifiers (type **`ID`** in the specification) are given as string
consisting of characters `[a-zA-Z0-9_.-]` of length at most 36 and not
starting with a `-` (dash) or `.` (dot) or ending with a `.` (dot). IDs are
Expand Down Expand Up @@ -1457,20 +1461,22 @@ The following endpoints are associated with state:

Properties of state objects:

| Name | Type | Description
| :--------------- | :----- | :----------
| started | TIME ? | Time when the contest actually started, or `null` if the contest has not started yet. When set, this time must be equal to the [contest](#contests) `start_time`.
| frozen | TIME ? | Time when the scoreboard was frozen, or `null` if the scoreboard has not been frozen. Required iff `scoreboard_freeze_duration` is present in the [contest](#contests) endpoint.
| ended | TIME ? | Time when the contest ended, or `null` if the contest has not ended. Must not be set if started is `null`.
| thawed | TIME ? | Time when the scoreboard was thawed (that is, unfrozen again), or `null` if the scoreboard has not been thawed. Required iff `scoreboard_freeze_duration` is present in the [contest](#contests) endpoint. Must not be set if frozen is `null`.
| finalized | TIME ? | Time when the results were finalized, or `null` if results have not been finalized. Must not be set if ended is `null`.
| end\_of\_updates | TIME ? | Time after last update to the contest occurred, or `null` if more updates are still to come. Setting this to non-`null` must be the very last change in the contest.
| Name | Type | Description
| :----------------- | :------------------ | :----------
| started | TIME ? | Time when the contest actually started, or `null` if the contest has not started yet. When set, this time must be equal to the [contest](#contests) `start_time`.
| frozen | TIME ? | Time when the scoreboard was frozen, or `null` if the scoreboard has not been frozen. Required iff `scoreboard_freeze_duration` is present in the [contest](#contests) endpoint.
| ended | TIME ? | Time when the contest ended, or `null` if the contest has not ended. Must not be set if started is `null`.
| thawed | TIME ? | Time when the scoreboard was thawed (that is, unfrozen again), or `null` if the scoreboard has not been thawed. Required iff `scoreboard_freeze_duration` is present in the [contest](#contests) endpoint. Must not be set if frozen is `null`.
| finalized | TIME ? | Time when the results were finalized, or `null` if results have not been finalized. Must not be set if ended is `null`.
| end\_of\_updates | TIME ? | Time after last update to the contest occurred, or `null` if more updates are still to come. Setting this to non-`null` must be the very last change in the contest.
| removed\_intervals | array of INTERVAL ? | Array of [removed time intervals](ccs_system_requirements#removing-time-intervals).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a requirement that intervals are strictly non-overlapping (also not "touching", since then they could be merged) and I'd say we should require the list to be sorted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Definitely agree with non-overlapping.
  • Not sure about "touching". On the one hand, what could the use possibly be to allow them? OTOH, it feels like overspecifying a bit.No every dumb thing should be explicitly forbidden.
  • Not sure about sorted, but I guess that's nice.

I'm certainly not against any of those improvements. Do you want to push a fix, or should I?


These state changes must occur in the order listed in the table above,
as far as they do occur, except that `thawed` and `finalized` may occur
in any order. For example, the contest may never be frozen and hence not
thawed either, or, it may be finalized before it is thawed. I.e., the
following sequence of inequalities must hold:
in any order. `removed_intervals` is not a state change, and so is not affected
by this requirement. For example, the contest may never be frozen and hence not
thawed either, or, it may be finalized before it is thawed. I.e., the following
sequence of inequalities must hold:

```
started < frozen < ended < thawed < end_of_updates,
Expand All @@ -1497,6 +1503,10 @@ Returned data:
"thawed": null,
"finalized": null,
"end_of_updates": null
"removed_intervals": [
"2014-06-25T10:30:00+01/10:45:00",
"2014-06-25T10:50:00+01/10:55:00"
]
}
```

Expand Down
1 change: 1 addition & 0 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ These are the main changes made since the `2023-06` version:
- Changed type of time related properties in scoreboard endpoint from `integer` to `RELTIME`.
- Added `current` to judgements endpoint.
- Added `country_subdivision` and `country_subdivision_flag` to organizations endpoint.
- Added `removed_intervals` to state endpoint.

## References

Expand Down