Skip to content

Commit

Permalink
Add packaging as explicit dependency and tweak flux adapter version c…
Browse files Browse the repository at this point in the history
…hecking (#427)

* Adds packaging to toml file as it is now a required non-dev dependency.  Version is constrained to >=22.0 to reflect removal of LegacyVersion which can be returned from parse_version in older packaging versions.
* Improve robustness of flux adapter version error handling by testing against base_version to allow pre-release builds to succeed.
* Add version comparison unit tests for both full and base_version variants
  • Loading branch information
jwhite242 authored Aug 23, 2023
1 parent 3283b01 commit 0442e1e
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 6 deletions.
1 change: 1 addition & 0 deletions docs/Maestro/scheduling.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ Flux adapter also supports some keys that control batch job behavior instead of
| **Maestro** | **Description** | **Default** |
| :- | :- | :- |
| `nested` | Flag to control whether to run the step inside a nested flux instance. This is usually the desired option. | True |
| `waitable` | Whether to mark a job as 'waitable'; this is restricted to owners of an instance, and thus cannot be used if scheduling to a system instance (i.e. not to a broker with a specific uri). Note: this option is likely only of interest if using the script adapters directly to build a custom tool. New flag as of 0.49.0 adapter. Let us know via [github issues](https://github.com/LLNL/maestrowf/issues) if you find a need/use for this in the spec. | False |

See the [flux framework](https://flux-framework.readthedocs.io/en/latest/index.html) for more information on flux. Additionally, checkout the [flux-how-to-guides](how_to_guides/running_with_flux.md) for the options available for using flux with Maestro. Also check out a [full example spec run with flux](specification.md#full-example).

Expand Down
7 changes: 5 additions & 2 deletions maestrowf/abstracts/interfaces/flux.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,25 @@ def connect_to_flux(cls):
if not versions_parsed:
return

if adaptor_version > broker_version:
if adaptor_version.base_version > broker_version.base_version:
LOGGER.error(
"Maestro adapter version (%s) is too new for the Flux "
"broker version (%s). Functionality not present in "
"this Flux version may be required by the adapter and "
"cause errors. Please switch to an older adapter.",
adaptor_version, broker_version
)
elif adaptor_version < broker_version:
elif adaptor_version.base_version < broker_version.base_version:
LOGGER.debug(
"Maestro adaptor version (%s) is older than the Flux "
"broker version (%s). This is usually OK, but if a "
"newer Maestro adapter is available, please consider "
"upgrading to maximize performance and compatibility.",
adaptor_version, broker_version
)
# TODO: add custom version object to more properly handle dev
# and prerelease versions for both semver and pep440 version
# schemes. Then add log message reflecting it if detected

@classmethod
def get_flux_version(cls):
Expand Down
6 changes: 3 additions & 3 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[tool]
[tool.poetry]
name = "maestrowf"
version = "1.1.10dev3"
version = "1.1.10dev4"
description = "A tool to easily orchestrate general computational workflows both locally and on supercomputers."
license = "MIT License"
classifiers = [
Expand Down Expand Up @@ -40,6 +40,7 @@ dill = "*"
filelock = "*"
importlib_metadata = {version = "*", python = "<3.8"}
jsonschema = ">=3.2.0"
packaging = ">=22.0"
pyyaml = ">=4.2b1"
rich = "*"
six = "*"
Expand Down
27 changes: 27 additions & 0 deletions tests/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,30 @@ def test_parse_version(version_string, expected_version, error):
with error:
version_parts = parse_version(version_string)
assert version_parts == expected_version


@pytest.mark.parametrize(
"test_version_string, ref_version, expected, base_expected",
[
("0.49.0", Version("0.49.0"), True, True),
("0.50.0rc2", Version("0.49.0"), True, True),
("0.49.0-225-g53e087510", Version("0.49.0"), True, True),
("0.48.0", Version("0.49.0"), False, False),
("0.49.0rc1", Version("0.49.0"), False, True),
],
)
def test_version_greater(test_version_string, ref_version, expected, base_expected):
"""
Test version comparison between variants of flux core's version strings
and Maestro's flux verison adapters to ensure correct adapter version
selection and error handling. Tests raw comparisons as well as fallback
to base_version for ignoring dev/pre-release variants
"""
test_version = parse_version(test_version_string)
ver_cmp = test_version >= ref_version
print(f"Version '{test_version}': base = '{test_version.base_version}', is prerelease = '{test_version.is_prerelease}'")

assert ver_cmp == expected

ver_cmp_base = test_version.base_version >= ref_version.base_version
assert ver_cmp_base == base_expected

0 comments on commit 0442e1e

Please sign in to comment.