-
-
Notifications
You must be signed in to change notification settings - Fork 652
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
daemon state invalidation strategy for e.g. pants.ini modification #3941
Comments
There are two categories here I think.
|
### Problem The "rule triple" / "task triple" syntax generally requires ~30 minutes worth of explanation for every new user. In particular, the triples _look_ like they are decoupled from individual functions, when they are generally defined 1-to-1. ### Solution Switch the majority of rule declarations to a new `@rule` decorator which creates and annotates the function with a `TaskRule`. The decorator is usable in all cases where constraints are known statically... in about three cases (where constraints are computed dynamically: ie, types from the `SymbolTable`) it is necessary to explicitly create a `TaskRule`. Additionally, removed `SelectLiteral` in favor of usage of `SingletonRule`s in all cases. When a `SingletonRule` is declared, callers can directly use `Select` to request the value for the singleton. Just like `SelectLiteral`, these continue to be an escape hatch for values which are not constructed from the ground up using files on disk (see #3941). ### Result Fixes #4289; "task triples" are gone: long live the `@rule` decorator!
Perhaps a little bit controversial, but I think that we can target this to |
### Problem The "rule triple" / "task triple" syntax generally requires ~30 minutes worth of explanation for every new user. In particular, the triples _look_ like they are decoupled from individual functions, when they are generally defined 1-to-1. ### Solution Switch the majority of rule declarations to a new `@rule` decorator which creates and annotates the function with a `TaskRule`. The decorator is usable in all cases where constraints are known statically... in about three cases (where constraints are computed dynamically: ie, types from the `SymbolTable`) it is necessary to explicitly create a `TaskRule`. Additionally, removed `SelectLiteral` in favor of usage of `SingletonRule`s in all cases. When a `SingletonRule` is declared, callers can directly use `Select` to request the value for the singleton. Just like `SelectLiteral`, these continue to be an escape hatch for values which are not constructed from the ground up using files on disk (see pantsbuild#3941). ### Result Fixes pantsbuild#4289; "task triples" are gone: long live the `@rule` decorator!
options parsing stuff makes sense. would like to chat about your thoughts on the implementation strategy for that a bit more whenever you have time @stuhood. it also seems like there's a blocking chicken&egg situation here in #4618, so I'll probably focus on that aspect for now. additionally, runtime imports will hold unwanted state in the form of stale pants sources which could lead to confusion when actively developing on pants. we'll likely need to devise a fingerprinting strategy to ensure the daemon stays in sync when it's underlying sources are modified - or else advocate for a development process that either excludes pantsd or emphasizes manual lifecycle control. |
.@benjyw volunteered to do the portion of this that is related to computing configuration using the engine. Thanks Benjy! |
now that pantsd's transitive options dependencies are in bootstrap options and the daemon launches from a clean memory state, I think the core problem here should be as easy as fingerprinting a specific set of bootstrap options and comparing that to a resident fingerprint in the running daemon. the fingerprint would include:
I /think/ we'll only have to worry about task and subsystem options as we begin to move their execution/initialization/storage into the v2 scheduler. |
(or it may be simpler to just fingerprint all bootstrap options) |
Problem Currently, if options (from cli args, pants.ini, env vars, rc files et al) affecting the daemon change between runs the daemon does not detect this and will happily run with trapped, stale state. Solution Fingerprint the relevant subset of bootstrap options at startup time in the thin client and compare that fingerprint to the one used at launch time for the current pantsd process to determine if the daemon needs to be restarted in advance of the run. This piggy-backs on top of the daemon externalization work. Result Fixes #3941.
Problem Currently, if options (from cli args, pants.ini, env vars, rc files et al) affecting the daemon change between runs the daemon does not detect this and will happily run with trapped, stale state. Solution Fingerprint the relevant subset of bootstrap options at startup time in the thin client and compare that fingerprint to the one used at launch time for the current pantsd process to determine if the daemon needs to be restarted in advance of the run. This piggy-backs on top of the daemon externalization work. Result Fixes pantsbuild#3941.
currently, modification to
pants.ini
in a given repo doesn't end up invalidating anything in the daemon. if pertinent new configuration is added (e.g. new ignore paths), stale data (e.g. errors in previously unignored paths) can be cached and served by the daemon.we should probably implement specialized handling for modifications to pants.ini to improve this situation. the same might also be true for
PANTS_*
environment variables.The text was updated successfully, but these errors were encountered: