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

Change default task expiration to None #980

Open
inahga opened this issue Apr 15, 2024 · 1 comment
Open

Change default task expiration to None #980

inahga opened this issue Apr 15, 2024 · 1 comment

Comments

@inahga
Copy link
Contributor

inahga commented Apr 15, 2024

Setting a hard expiration of 1 year is not very useful. We should set the default expiry to None and let the user control the expiration. We should probably also migrate existing tasks to have no expiry.

See https://isrg.slack.com/archives/C0167LT4C73/p1713204773121299 for discussion.

@branlwyd This question is complicated. IMO, there are several competing factors.
Points for longer task expirations:

  1. Subscribers will not like the toil of having to recreate the "same" tasks -- this is a chore that has no upside for them, which will cause their system to grind to a halt if they forget to do it.
    Points against longer task expirations:
  2. We need some way to cause subscribers to update to the newest version of DAP. Task expiration might be a good stick.
  3. This might force subscribers to rotate the secrets associated with each task.
    However, I think this second point is not so good: for white-glove integrations, we can just ask the subscriber to update. For eventual self-service subscribers in the shared service, we would do the standard deprecation dance of "turn on new functionality, announce end date for old functionality, wait, disable old functionality". That is, I don't think task expiration is necessary, nor is it the best tool, to enforce protocol version upgrades.
    The third point, IMO, is also not so good: the best analogy I've seen is to TLS certificates, which use time-based expiration. However: there is no requirement to actually rotate the key stored in the certificate. I believe time-based expiration is mostly a requirement due to the "distributed" nature of the CA ecosystem, which does not have a good revocation story. Since in this context, our DAP tasks are centrally managed (by us & our subscribers), we don't need to use time-based revocation; and IMO it's better to rotate secrets when there is evidence of compromise.
    All that said: I think ultimately we may want to remove task expiration completely, to ease usage for our subscribers.
    Thoughts? (edited)

@inahga I agree on all points.
Cert expiry is a notorious timebomb which is only made tolerable by automation. What automation could we have around task expiration that doesn't effectively amount to bumping the date? Maybe rotating secrets--but all secrets in play are internal so they should be implementation details.
Removal shouldn't require Janus changes either. task_expiration field would turn into a tombstone. In fact, even the divviup-api changes would be minimal since similarly expiration would just be a tombstone.

User control of expiration might be desirable. If a user has a timeboxed experiment that they're running, they may wish to timebomb the task so that it doesn't accrue costs after they're done with the experiment.
So I think some notion of a timebomb is desirable.
The context for this question is that I'm wiring up #862 which involves plumbing through allowing updates of task expiration, so I wanted to know what validation would go around the API call.
If we wanted to do away with task expiration entirely, I suppose no validation of the time is necessary. If you want it to expire January 1st, 1970 or now(), the task is disabled. If you want to disable it 1 day/week/year from now, so be it.

@branlwyd Yeah, I suppose I should amend my conclusion from "we should remove task expiry" to "task expiry's default should be 'never'".

@branlwyd
Copy link
Member

editorial: points 2 & 3 are points against a longer task expiration.

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