pantsbuild #development

i guess one approach would be to start prefetching everything behind the root snapshots in the workspace concurrently with starting the process to consume it?

witty-crayon-22786

04/17/2018, 11:28 AM

...and then "never" implement concurrent reads?

witty-crayon-22786

04/17/2018, 11:28 AM

...or implement reads concurrently as a cheap in-memory lookup

witty-crayon-22786

04/17/2018, 11:29 AM

cheap/blocking

witty-crayon-22786

04/17/2018, 11:30 AM

ok, sorry. reading.

happy-kitchen-89482

04/17/2018, 2:56 PM

@witty-crayon-22786 nice

average-vr-56795

04/17/2018, 3:55 PM

@aloof-angle-91616 Reviewing https://github.com/pantsbuild/pants/pull/5703 I think in the future we’re going to want a new thing that looks a lot like

datatype

but which takes a map of name -> type and automatically does the isinstance checking for you

average-vr-56795

04/17/2018, 3:56 PM

class CatExecutionRequest(typeddatatype('CatExecutionRequest', [('shell_cat_binary', ShellCat), ('input_file_globs', PathGlobs)])

or something

average-vr-56795

04/17/2018, 3:57 PM

Because there’s so much boilerplate in all of those checks which we’re clearly going to want everywhere

average-vr-56795

04/17/2018, 3:57 PM

(But also, maybe we should just ditch python for a language with types? Kidding… Kidding…)

average-vr-56795

04/17/2018, 4:01 PM

I also suspect that we’re going to want some kind of function for “I’m just invoking a process with some args, make it all for me”, because needing to define: class Concatted class ShellCat class CatExecutionRequest def cat_files_process_request_input_snapshot def cat_files_process_result_concatted def create_cat_stdout_rules just to write the semantic equivalent of bash’s

Copy code

output="$(cat "${input_files[@]}")"
exit_code=$?

or python’s

subprocess.check_output(["cat"] + input_files)

doesn’t feel great ergonomically… (But this is how we work that out 🙂)

average-vr-56795

04/17/2018, 4:03 PM

Also, I continue to be super impressed and happy with this PR 😄

aloof-angle-91616

04/17/2018, 4:18 PM

the type checking absolutely needs to be automated and the only reason i haven’t implemented it with a name and type dict is because i immediately fall into a mental rabbit hole regarding combining rules but instance checking is really, really easy to do and would be incredibly useful i think

aloof-angle-91616

04/17/2018, 4:20 PM

and i agree that making a process invocation is a little unnatural as of now and believe that the answer may involve doing things in the Binary class and making those things highly composeable

average-vr-56795

04/17/2018, 4:21 PM

Yeah 🙂 That ^^ is all definitely random thoughts for the future, and not stuff to do right now

aloof-angle-91616

04/17/2018, 4:21 PM

i also have really been trying to implement the modification i mentioned in the pr to allow single Gets instead of having to make one to get the ExecuteProcessRequest and another to get the ExecuteProcessResult and having major trouble so gonna table that for now too

average-vr-56795

04/17/2018, 4:21 PM

Left some comments which basically all come down to “delete a bunch more stuff”, but approved the PR 🙂 Let’s get it in, and move on to some fun fun follow-ups!

aloof-angle-91616

04/17/2018, 4:22 PM

i agree on that, but instance checking in datatypes imo should happen

aloof-angle-91616

04/17/2018, 4:22 PM

i would love to delete things and get this merged so thanks a lot

aloof-angle-91616

04/17/2018, 4:23 PM

i’m gonna play around with that a bit after standup and see if i can implement the instance checking concisely

aloof-angle-91616

04/17/2018, 4:23 PM

but after this is merged focusing on native stuff this week probably

average-vr-56795

04/17/2018, 4:24 PM

Sounds good 🙂

average-vr-56795

04/17/2018, 4:24 PM

Will cut some tickets for the follow-up stuff, so that anyone can grab them when they find time 🙂

aloof-angle-91616

04/17/2018, 4:25 PM

that’d be great, let me know if i can flesh any of those out (i still need to add docs for RootRule usage)