Skip to content
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

[hailctl] batch submit fixes #14351

Closed
wants to merge 26 commits into from

Conversation

jigold
Copy link
Contributor

@jigold jigold commented Feb 23, 2024

CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.

Fixes #14274

Replacing PR #14186

Dan King and others added 22 commits February 7, 2024 12:31
Issues resolved herein:

1. build.yaml tests must not use `exit 0` as it exits the test early.
2. Always prefer `orjson` to `json`.
3. Add `--wait` which waits for the submitted batch to complete and exits success only when the batch is success.
4. Whenever working with paths, we must use the `realpath` which resolves symlinks. In particular,
   on Mac OS X, `/tmp` is a symlink to `/private/tmp` and Python's APIs are inconsistent on whether
   they return a realpath or a path with symlinks. [1]
5. If the destination looks like a directory (e.g. "bar:/foo/", "bar:/"), the tests all suggest we
   should copy *into* not *to*. We now check for a trailing slash and copy *into*.
6. `ln -s src dst` means different things depending on whether dst is an extant folder or not. In
   this PR, I prefer to always be fully explicit so I never rely on `ln` detecting the destination
   is a directory and acting differently. Put differently: `file_input_to_src_dest` now never
   returns a file source and a destination folder.
7. We need to create the `real_absolute_cwd()` on the job before we `cd` into it.
8. `test_dir_outside_curdir` suggests that `--file foo/:/` is meant to copy the contents of foo into
   the root. This cannot be implemented with our symlink strategy (you can't replace the root with a
   symlink), so I changed the interpretation: a trailing slash on the source is meaningless. If the
   destination ends in a slash, we "copy into", otherwise we "copy to".
9. Add examples of --files usage.

[1]:

```ipython3
In [1]: import tempfile
   ...: tempfile.TemporaryDirectory()
Out[1]: <TemporaryDirectory '/var/folders/x1/601098gx0v11qjx2l_7qfw2c0000gq/T/tmp_pmj3lr9'>

In [2]: import os
   ...: os.getcwd()
Out[2]: '/private/tmp'

In [3]: !ls -al /tmp
lrwxr-xr-x@ 1 root  wheel  11 Aug  2 05:44 /tmp -> private/tmp
```
…re missing config.ini file, user friendly error messages when script file or --files files do not exists
@jigold
Copy link
Contributor Author

jigold commented Mar 15, 2024

@iris-garden

@daniel-goldstein daniel-goldstein removed their assignment Jul 22, 2024
@chrisvittal chrisvittal requested a review from ehigham January 28, 2025 19:34
ehigham added a commit to ehigham/hail that referenced this pull request Jan 30, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
@ehigham ehigham closed this Jan 30, 2025
ehigham added a commit to ehigham/hail that referenced this pull request Jan 30, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Jan 30, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Jan 30, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Jan 31, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 3, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
ehigham added a commit to ehigham/hail that referenced this pull request Feb 7, 2025
CHANGELOG: Fix many issues, including (hail#14274), with hailctl batch submit introduced in 0.2.127.
Fixes hail-is#14274, Replaces hail-is#14351 (authored by @jigold)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants