Skip to content
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.

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

Add a "play to time" feature to the HistoricalModel API #240

Closed
mmacfadden opened this issue Jul 11, 2021 · 4 comments
Closed

Add a "play to time" feature to the HistoricalModel API #240

mmacfadden opened this issue Jul 11, 2021 · 4 comments
Assignees
Labels
client Issues related to the client enhancement New feature or request proto server Issues related to the server
Milestone

Comments

@mmacfadden
Copy link
Contributor

mmacfadden commented Jul 11, 2021

Versions

  • Convergence Version: 1.0.0-rc.9

Description
Presently, you can only play to a version in the HistoricalModel class. It would be helpful to be able to play back the model to a certain point in time. However, it is possible that a version does not explicitly correspond to a particular wall time. Assume the following operations timestamps (all on the same day):

  1. Op1: 1:00pm
  2. Op2: 1:05pm
  3. Op3: 1:07pm

Requesting to play to any time before 1pm or after 1:07pm would be an error condition. Users of the model do know time bounds of the model, so we will expect them to use those as proper bounds. Asking to play back to 1:05pm would bring us to the document with operations 1 and 2 applied. If a valid time (within the min / max bounds) is requested, but that does not correspond to an operation timestamp. The model will be played back to the previous operation. For example if 1:02pm was requested, the model would be at the document version that only has Op1 applied.

Here is the proposed API:

public playToTime(time: Date): Promise<void>;

The following protocol messages will be added:

message GetVersionAtTimeRequestMessage {
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.RequestMessage";
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.HistoricalModelMessage";
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.ClientMessage";

  string modelId = 1;
  google.protobuf.Timestamp targetTime = 2;
}

message GetVersionAtTimeResponseMessage {
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.ResponseMessage";
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.HistoricalModelMessage";
  option (scalapb.message).extends = "com.convergencelabs.convergence.proto.ServerMessage";

  int64 versionAtTime = 1;
}
message ConvergenceMessage {

  ...

  oneof body {
    
    ...

    model.GetVersionAtTimeRequestMessage modelGetVersionAtTimeRequest = 143;
    model.GetVersionAtTimeResponseMessage modelGetVersionAtTimeResponse = 144;

    ...
@mmacfadden mmacfadden added enhancement New feature or request server Issues related to the server client Issues related to the client proto labels Jul 11, 2021
@mmacfadden mmacfadden added this to the 1.0.0-rc.10 milestone Jul 11, 2021
@mmacfadden mmacfadden self-assigned this Jul 11, 2021
@mmacfadden mmacfadden changed the title A "play to time" to the HistoricalModel API Add a "play to time" feature to the HistoricalModel API Jul 11, 2021
@toebes
Copy link

toebes commented Jul 11, 2021

If I create the model at 10am and the first time that anyone modifies the model is 3pm, then I would expect that doing a playto 2:49pm would bring me to the initial version of the model.

Attempting to play to before the model was created is ok to be an error and that should be the only error.

Even if you attempt to play to before the model was created, despite the thrown error, it should be at the first version of the model.

As for going beyond the end of the model, any attempt to jump to any time past the last modified time of the model should go to the last version of the model.

@mmacfadden
Copy link
Contributor Author

Ok. I agree that handling any time after the model was created is a reasonable request. I do not want to handle the case of before the model. When you open the historical model, it tells you what the minimum valid time is. There is not really a good reason to ask for a time before that, in my opinion. this is something the developer can easily check for or enforce themselves if they want that behavior.

@toebes
Copy link

toebes commented Jul 11, 2021

I agree.

mmacfadden added a commit to convergencelabs/convergence-proto that referenced this issue Jul 11, 2021
mmacfadden added a commit to convergencelabs/convergence-server that referenced this issue Jul 11, 2021
mmacfadden added a commit to convergencelabs/convergence-client-javascript that referenced this issue Jul 11, 2021
@mmacfadden
Copy link
Contributor Author

This has been implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Issues related to the client enhancement New feature or request proto server Issues related to the server
Projects
None yet
Development

No branches or pull requests

2 participants