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

Investigation: update experiments as part of the REST API (TSP) #608

Open
bhufmann opened this issue Dec 16, 2021 · 5 comments
Open

Investigation: update experiments as part of the REST API (TSP) #608

bhufmann opened this issue Dec 16, 2021 · 5 comments

Comments

@bhufmann
Copy link
Collaborator

Triggered by the server update proposal [1] and the TSP specification for that [2] some design thinking needs to decide the way forward. Updates in TSP, server and front-end will triggered based on the decision made for this.

Background:

The TSP specification [2] proposes a PUT command on the tsp/experiments/{expUUID} endpoint for the following use cases:

  • Rename experiment
  • add or remove of traces to an experiment
  • other updates (e.g. offsetting of traces)

The implementation of a PUT command requires to be idempotent, which means calling it once or several times successively has the same effect. This means the UUID cannot change for the experiment.

The current Trace Compass server implementation, however, creates the UUID based on the experiment name and persists the experimented as folder resource in the server workspace. Renaming an experiment will require to change the UUID. However, this would break the idempotent requirement.

Moreover, it is not clear if adding or removing of traces should be seen as the same experiment. If yes, then UUID cannot change and a PUT command can be used. If it cannot be seen as a same experiment, then a POST command would be more appropriate.

This ticket is to discuss the way forward for updating of experiments. Based on the outcome, issue trackers and updates will be triggered in relevant components.

[1] https://git.eclipse.org/r/c/tracecompass.incubator/org.eclipse.tracecompass.incubator/+/187742
[2] https://github.com/theia-ide/trace-server-protocol/blob/34d244cc35e652681b2f003d3bd69100da6541d6/API.yaml#L216

@bhufmann
Copy link
Collaborator Author

bhufmann commented Feb 11, 2022

PUT commands require to be idempotent, multiple requests have to have the same outcome. For a PUT to be idempotent, it is required to provide all parameters that makes up an experiment to be sent in the PUT command, e.g. name and traces.

PATCH commands don't have that requirement of having to be idempotent. They can be idempotent, but don't have to. It can be used to update part of the experiment resource.

For both PUT and PATCH, the experiment UUID must not change. The Trace Server implementation needs to be able to provide that.

Rename

For renaming of the experiment I think PATCH would be more suitable because only the to be modified name needs to be provided.

Add/Remove Traces

Adding and removing of traces in an experiment will affect the start/end time, available views and other capabilities. The front-end will need to be fully refreshed and any invalid views need to be removed. I would argue to close the experiment before doing this action.
Alternative 1)
This action can currently already be achieved by deleting the existing experiment and creating a new experiment with a new list of traces. Added traces will have to be uploaded first, removed traces need to be removed from the server. This will add more responsibility on the clients, while the server side implementation is simpler.
Alternative 2)
PATCH command on tsp/experiments/{expUUID}. This will update the experiment on the server. More responsibility and complexity is added to the server for handling that. Added traces will have to be uploaded first, while removed traces will be removed automatically by the server (if not in use otherwise)

Other updates (e.g. offsetting of traces)

Generally, other updates, e.g. change of experiment type, can be done using a PATCH command.

Offsetting traces in an experiment is useful to align the time from different traces.
Use a PATCH command to provide an offset for each trace in the experiment. Providing a offset to a trace for given traceUUID will affect other experiments that include the same trace with the same UUID. To avoid that the offset of a given trace affects other experiments that contains the same trace a copy of the trace needs to be done that creates a new UUID. On the server, a symbolic link to the original trace file should be used to avoid costly copies.
Alternative 1)
The copy of the trace and offset configuration is done by the client. The experiment is updated with the copied trace. This will add more responsibility on the clients, while the server side implementation is simpler.
Alternative 2)
PATCH command on tsp/experiments/{expUUID}. The sever takes care of the copy and offsetting of the traces. More responsibility and complexity is added to the server for handling that.

Notes

The trace server implementation have to be able to handle the modification of the experiment concurrently with other clients sending requests towards the same experiment instances (same UUID).

Implementation approach

Implement each feature incrementally to be able to focus on one feature at a time (full-stack implementation). For each feature update API specification, server implementation, tsp-typescript-client/tsp-python-client and then front-end.

  1. Rename
  2. Add/Remove
  3. Time Offsetting

@marco-miller
Copy link
Contributor

I think it would be worth it to benefit from the recently introduced ADR structure (#638),

  • as we formalize this well-progressing investigation further.
  • As this experiments-related case could pave the way for other features with potentially similar requirements.
  • This could also lead to some more GraphQL (mutations) related requirements.

@bhufmann
Copy link
Collaborator Author

I think it would be worth it to benefit from the recently introduced ADR structure (#638),

* as we formalize this well-progressing investigation further.

* As this experiments-related case could pave the way for other features with potentially similar requirements.

* This could also lead to some more `GraphQL` (mutations) related requirements.

I can add an ADR for this. Will give me a chance to try it out myself. Thanks for the suggestion.

bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Feb 22, 2022
bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Mar 11, 2022
bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Mar 22, 2022
bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Mar 22, 2022
bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Mar 25, 2022
@bhufmann
Copy link
Collaborator Author

#688 adds an ADR for renaming of an experiment and add/remove of traces in an experiment. Other updates, e.g. offsetting of traces, are not part of the ADR. I'll create a separate tracker for offsetting of traces.

bhufmann added a commit to bhufmann/theia-trace-extension that referenced this issue Mar 28, 2022
bhufmann added a commit that referenced this issue Mar 29, 2022
@bhufmann
Copy link
Collaborator Author

bhufmann commented Nov 3, 2023

I have another suggestion that is not reflected in the ADR as of today, yet. I think we could use custom command suffixes to define endpoints to do different behaviours, like rename experiment, add/remove traces from experiments. Use POST, PUT or PATCH depending on action. POST makes sense for rename because it moves one resource to a new one:

  • POST ${baseUrl}/experiments/{expUUID}:rename
  • POST ${baseUrl}/experiments/{expUUID}:add
  • POST ${baseUrl}/experiments/{expUUID}:remove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants