[batch] re-allow sending kwargs to python jobs #13505
Conversation
CHANGELOG: Fix bug introduced in 0.2.117 by commit `c9de81108` which prevented the passing of keyword arguments to Python jobs. This manifested as "ValueError: too many values to unpack".
hail/python/hailtop/batch/job.py
Outdated
| @@ -877,6 +877,19 @@ async def _compile(self, local_tmpdir, remote_tmpdir, *, dry_run=False): | |||
| return True | |||
|
|
|||
|
|
|||
| UnpreparedArg = '_resource.ResourceType' | List['UnpreparedArg'] | Tuple['UnpreparedArg', ...] | Dict[str, 'UnpreparedArg'] | Any | |||
There was a problem hiding this comment.
I think we have to wait until 3.10 for this unfortunately.
danking
force-pushed
the
fix-kwargs-in-python-jobs
branch
from
1cb6323
to
f5755b8
Compare
| @@ -448,3 +448,6 @@ def __str__(self): | |||
|
|
|||
| def __repr__(self): | |||
| return self._uid # pylint: disable=no-member | |||
|
|
|||
|
|
|||
| ResourceType = PythonResult | ResourceFile | ResourceGroup | |||
There was a problem hiding this comment.
Still have pipes here. I didn't mean to get rid of the types altogether though, just replace the pipes with a Union
|
Ah, my bad, I thought you mean the recursive types with strings. I didn't realize the pipes were new. That's what I get for not clicking links. |
This reverts commit 19807da.
danking
dismissed
daniel-goldstein’s stale review
merge main, actually address comments, ensure pyright is satisfied
| @@ -12,15 +12,17 @@ def to_dict(self): | |||
|
|
|||
There was a problem hiding this comment.
I don't entirely understand why pyright runs on everything but it does and it dislikes the formatting of this file.
| @@ -5,7 +5,7 @@ | |||
|
|
|||
| from hailtop.aiocloud.aiogoogle import get_gcs_requester_pays_configuration | |||
| from hailtop.aiocloud.aiogoogle.user_config import get_spark_conf_gcs_requester_pays_configuration, spark_conf_path | |||
| from hailtop.config.user_config import ConfigVariable, configuration_of | |||
| from hailtop.config import ConfigVariable, configuration_of | |||
There was a problem hiding this comment.
pyright also disliked this
There was a problem hiding this comment.
and it's right: ConfigVariable is not part of user_config.py
CHANGELOG: Fix bug introduced in 0.2.117 by commit
c9de81108which prevented the passing of keyword arguments to Python jobs. This manifested as "ValueError: too many values to unpack".We also weren't preserving tuples. They become lists. I fixed that too. I also added some types and avoided an is instance by encoding the necessary knowledge.