search

CarnegieLearningWeb / UpGrade

Framework for adding A/B testing to education applications
https://www.upgradeplatform.org/
BSD 3-Clause "New" or "Revised" License
25 stars 12 forks source link

Context Metadata / Create an API to change context metadata #62

Open nirmalpatel opened 3 years ago

nirmalpatel commented 3 years ago

Right now, the context metadata (experiment points and IDs that are valid, and a few other things) come from the environment file of the backend (see #26 for more details).

We need to create a CRUD API for experiment points and IDs and store the allowed values in the DB so that they can be added/removed without restarting the backend.

Questions:

  1. Should we allow deleting legal point/ID values from the backend if they are in use? Probably not?
  2. Should we create a UI in UpGrade to do this (like we add remove metrics from the UpGrade UI)?
SritterCL commented 3 years ago

My view of the context metadata is that it's a contract from the app saying that the app code supports certain experiment points. That's why it makes sense to make this an API (a command from the app), rather than environment files on the UpGrade side.

So, for #1, if the app says that an experiment point no longer exists in the code, but there's an active experiment that's using that experiment point, we've got a serious problem. UpGrade should definitely log an error (and return an error as a result of the API call). UpGrade shouldn't delete or inactivate the experiment, though. Either the app or the UpGrade side is wrong about whether the experiment point exists. If UpGrade is wrong, it'll send conditions for an experiment, but the experiment won't be able to be run. That's bad, but users will just get the default condition, which is all the app can provide anyway. If the app is wrong (and the experiment point really does exist in the app code), then the experiment should run fine.

For #2, I don't think so. Part of the problem this is trying to solve is making sure that UpGrade and the app are in sync with respect to what experiment points are currently supported. Experimenters shouldn't fix "experiment point does not exist" warnings by changing this metadata. They should make sure that the experiment points actually exist in the app (so they should talk to the app team).

msandbothe commented 3 years ago

@YashilDepani Is this resolved by #80 or is there more to do?

VivekFitkariwala commented 3 years ago

No this is not resolved by #80. We need extra work for this.

nirmalpatel commented 3 years ago

@SritterCL @amurphy-cl I guess we only allow to delete point/ID combinations when we don't have any associated experiments that are active, right?

Also makes me think about the archive status...

For now, if there are experiments on point/ID that someone is trying to delete, we can throw an error and give them back a list of the experiments that they need to cancel/delete before they can delete the exp point/ID.