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 the concept of a "readiness" similar to k8s, etc. #1294

Open
alecthomas opened this issue Apr 18, 2024 · 3 comments
Open

Add the concept of a "readiness" similar to k8s, etc. #1294

alecthomas opened this issue Apr 18, 2024 · 3 comments

Comments

@alecthomas
Copy link
Collaborator

@mistermoe and I chatted about this

Could be something like this:

//ftl:export
func Readiness(ctx context.Context) error {
	_, err := bearerDID.Get(ctx)
	return err
}
@github-actions github-actions bot added the triage Issue needs triaging label Apr 18, 2024
@alecthomas alecthomas removed the triage Issue needs triaging label Apr 18, 2024
@alecthomas alecthomas added the triage Issue needs triaging label May 20, 2024
@deniseli deniseli added next Work that will be be picked up next and removed triage Issue needs triaging labels May 21, 2024
@deniseli
Copy link
Contributor

Leaving some quick questions here for whoever ends up picking this up:

  1. Should we require the function to be named Readiness or should we rely on an //ftl:readiness directive instead? If so, do we want Readiness to be a reserved name? i.e. no other functions can be named that.
  2. Do we want to perform the readiness check (i.e. call each exported Readiness func) in Service.Status() in controller.go?
  3. Should readiness effectively be a specialty verb? i.e. if a function is exported with the readiness directive, and only that directive, then it also automatically gets treated as a verb in all the ways a verb would be; it's just also invoked for the readiness checks at startup? It's a bit odd because by the time the module is live, any Readiness function would presumably be returning nil persistently, so there wouldn't really be a point to calling it.
    1. Related: is there any reason to make these exportable? For module-to-module calls, it seems like exporting is just not useful. For calling it from the controller, we could argue that's a public/external access, in which case the visibility should technically be public, but that's basically implied already by the nature of readiness.
    2. tl;dr: I'm leaning towards: Readiness is not a verb. It needs directive //ftl:readiness precisely. No explicit export in the directive. No module-to-module calls. The controller will call this function at startup.

@alecthomas
Copy link
Collaborator Author

I agree re. not being exported, I think //ftl:export in the example predated visibility controls.

I think it should be a verb for consistency though, as that is our unit of work for everything in FTL. I'm fine with either a well known name or //ftl:readiness, though the latter is more explicit.

@deniseli
Copy link
Contributor

deniseli commented Jun 1, 2024

Sounds good! I'll pick this up after I finish my other issues if no one else beats me to it (but everyone definitely please feel free to take this!)

@deniseli deniseli self-assigned this Jun 6, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Jun 6, 2024
@deniseli deniseli removed their assignment Jun 6, 2024
@deniseli deniseli added the next Work that will be be picked up next label Jun 6, 2024
@matt2e matt2e removed the next Work that will be be picked up next label Jul 1, 2024
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

3 participants