https://pantsbuild.org/ logo
Join Slack
Powered by
Call-by-name issue: ```await Get(ProcessResult, V...
# development
h
Call-by-name issue:
Copy code
await Get(ProcessResult, VenvPexProcess(...))
completes in a few seconds as expected.
Copy code
await process_request_to_process_result(VenvPexProcess(...))
spins indefinitely.
Copy code
process = await Get(Process, VenvPexProcess(...))
await process_request_to_process_result(process)
completes in a few seconds as expected.
Should call-by-name invocations not still convert the input instance to the expected type via the solver?
@wide-midnight-78598 this is presumably what the migration script would convert to, so presumably it is supposed to work?
Oh, this does work:
Copy code
await process_request_to_process_result(
        **implicitly(VenvPexProcess(...)))
So now I'm mildly confused and wondering if the migration won't work as-is in such cases?
w
One of those examples should give a typecheck error, no?
As far as I knew, the latest (but forever improving) migration script does attempt to wrap the 
VenvPexProcess
with implicitly That example is almost exactly a test case: https://github.com/pantsbuild/pants/blob/f151aaae081886a1c409127fcca23daa84b76d20/src/python/pants/goal/migrate_call_by_name_test.py#L181
Copy code
"process_request_to_process_result(VenvPexProcess(arg1, arg2, arg3), **implicitly())",
"async def process_request_to_process_result(process: Process, process_execution_environment: ProcessExecutionEnvironment) -> FallibleProcessResult: ...",
"process_request_to_process_result(**implicitly(VenvPexProcess(arg1, arg2, arg3)))",
        ),
Note that the migration is in 2-steps: 1. Migrate from Get to CBN 2. Run the 
implictly
pseudo-fixer (which will eventually be pulled out into a flake8 or whatever) to try to clean up the aggressive implicitly usage (remove unneeded, modify other usage)
h
Gotcha, so it compares the exact type of the provided arg to the exact type of the rule arg and implicitly-s if they don't match? Or if there are other args?
w
Right now, the fixer only handle like, the 80% case - which is one arg (https://github.com/pantsbuild/pants/blob/f151aaae081886a1c409127fcca23daa84b76d20/src/python/pants/goal/migrate_call_by_name.py#L570) Basically, I think only dictionaries have more explicitly called args - so those are manually handled, but yeah: • If the number of called args matches the target function's arg count, then remove implicitly • If the arg types don't match, wrap with implicitly. • If everything looks right, leave the implicitly trailing arg alone
👍 1
The subsequently unhandled cases are multiple args, which like, yeah, that needs to get done to be complete - but in the migration, I think I've only run into 1 instance of it so far, there are probably dozens total
6 Views
Open in Slack
PreviousNext
Powered by