I think I could use a new `ProcessCacheScope` `PER RESTART I Pants #plugins

I think I could use a new `ProcessCacheScope`: `PE...

proud-dentist-22844

05/20/2021, 5:48 AM

I think I could use a new `ProcessCacheScope`:

PER_RESTART_IF_SUCCESSFUL

ie cache/memoize in memory, but only when it exits successfully. wdyt?

hundreds-father-404

05/20/2021, 5:57 AM

I think that makes a lot of sense. And probably change

PER_RESTART

to be

PER_RESTART_ALWAYS

PER_RESTART_SUCCESSFUL

👍 1

hundreds-father-404

05/20/2021, 1:47 PM

Ah ha. I realized why we didn't think to do this before. Everywhere

PER_RESTART

is used atm, we use

await Get(ProcessResult, Process)

instead of

await Get(FallibleProcessResult, Process)

. This causes Pants to error if return code != 0, which restarts Pantsd But your change is still an excellent suggestion. About to put up a PR to fix

proud-dentist-22844

05/20/2021, 1:59 PM

Cool. Checking for a running mongo instance is accessible would fit well with

PER_RESTART_SUCCESSFUL

🙂

hundreds-father-404

05/20/2021, 2:20 PM

Btw, is your plugin going to error if mongo is not accessible? FYI that will restart Pantsd if so But yeah, good change regardless, writing the PR description now

hundreds-father-404

05/20/2021, 2:35 PM

https://github.com/pantsbuild/pants/pull/12094

proud-dentist-22844

05/20/2021, 3:29 PM

Yeah. It errors if it is not accessible. Is there a kinder way to make pants abort the goal?

hundreds-father-404

05/20/2021, 3:30 PM

Not atm. I do agree that would be desirable - killing Pantsd is not a good side effect of raising an exception imo

It errors if it is not accessible.

Cool, so then you don't actually need #12094. Although, again, still a good change regardless

hundreds-father-404

05/20/2021, 3:33 PM

One thing I think you might want though is

PER_SESSION_{ALWAYS,SUCCESSFUL}

. Iiuc, per-restart means the lifetime of the pants daemon, which ideally lives for a while like multiple days A session is a call to

./pants

. Meaning,

./pants test ::

would run your rule once and memoize it across all your tests. But then calling

./pants test ::

again would re-run the process

proud-dentist-22844

05/20/2021, 3:34 PM

I assumed that pants would survive the error. 😛

proud-dentist-22844

05/20/2021, 3:34 PM

Ah yeah.

PER_SESSION_SUCCESSFUL

would be great.

hundreds-father-404

05/20/2021, 3:49 PM

I assumed that pants would survive the error.

Yeah, that's actually a really great suggestion. Opened https://github.com/pantsbuild/pants/issues/12096 for more graceful failure modes

hundreds-father-404

05/20/2021, 4:01 PM

https://github.com/pantsbuild/pants/issues/12097 for

PER_SESSION

. I'm not very clear on how to implement that one, although asked Stu if he could weigh in when he has a moment (he's on paternity leave)

proud-dentist-22844

05/20/2021, 4:02 PM

paternity leave - cool!

🐣 1

hundreds-father-404

05/20/2021, 7:50 PM