feat: add migration worker and update manage to have a singular migra…#1819
feat: add migration worker and update manage to have a singular migra…#1819ale8k merged 4 commits intocanonical:v3from
Conversation
SimoneDutto
left a comment
There was a problem hiding this comment.
looks good, a few comments here and there
SimoneDutto
left a comment
There was a problem hiding this comment.
one more test please and it's approved
| u := &dbmodel.Identity{Name: job.Args.Username} | ||
| if err := w.store.FetchIdentity(ctx, u); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
This looks good.
It would be awesome, if you feel like it, to refactor the permission checks so that they're in jujuapi and make it so that the manager doesn't need the OpenFGA user at all.
There was a problem hiding this comment.
How could we do it in jujuapi? Any worker could pick this up, and the time between their session could have been invalidated. Unless I'm misunderstanding?
There was a problem hiding this comment.
Can discuss separately another time.
There was a problem hiding this comment.
Yeah it's a bigger issue I think but I'm not really too sure on how we solve it... I think each call should re-validate what it intends to do in a smart way.
ab6ebc5 to
a1f9c24
Compare
| u := &dbmodel.Identity{Name: job.Args.Username} | ||
| if err := w.store.FetchIdentity(ctx, u); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Can discuss separately another time.
Description
…te method. This PR:
Engineering checklist
Test instructions