Skip to content

Commit

Permalink
Fixes for prediction API
Browse files Browse the repository at this point in the history
This PR resolves a couple bugs encountered during testing:

- package and compiler versions can now be made up of any non-space character. previously, the regex matched semvar formatting, but spackages can have versions in other formats
- clarified that specs should be URL-encoded in calls to /api/v1/allocate
- fixed issue where expensive variants algo was unable to complete due to some packages storing popular variants string values rather than boolean. ex: quantum-espresso hdf5=none
  • Loading branch information
cmelone committed Aug 21, 2024
1 parent 77e9f04 commit 32e451c
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 5 deletions.
2 changes: 2 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ The spec sent to the endpoint should have the following format:
pkg_name@pkg_version +variant1+variant2%compiler@compiler_version
```

Be sure that the string is URL-encoded. For instance, the `urllib.parse.quote` method will ensure the proper format. Without it, the allocation algorithm may return inaccurate results.

**There must be a space after the package version in order to account for variant parsing.**

If the request does not contain a valid spec, the API will respond with `400 Bad Request`. The maximum allowed size of the `GET` request is 8190 bytes.
Expand Down
7 changes: 5 additions & 2 deletions gantry/routes/prediction/prediction.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,17 @@ async def select_sample(query: str, filters: dict, extra_params: list = []) -> l
# iterate through all the expensive variants and create a set of conditions
# for the select query
for var in EXPENSIVE_VARIANTS:
if var in spec["pkg_variants_dict"]:
variant_value = spec["pkg_variants_dict"].get(var)

# check against specs where hdf5=none like quantum-espresso
if isinstance(variant_value, (bool, int)):
# if the client has queried for an expensive variant, we want to ensure
# that the sample has the same exact value
exp_variant_conditions.append(
f"json_extract(pkg_variants, '$.{var}')=?"
)

exp_variant_values.append(int(spec["pkg_variants_dict"].get(var, 0)))
exp_variant_values.append(int(variant_value))
else:
# if an expensive variant was not queried for,
# we want to make sure that the variant was not set within the sample
Expand Down
4 changes: 2 additions & 2 deletions gantry/tests/test_prediction.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ async def test_empty_sample(db_conn):
# Test validate_payload
def test_valid_spec():
"""Tests that a valid spec is parsed correctly."""
assert parse_alloc_spec("[email protected] +json+native+treesitter%[email protected]") == {
assert parse_alloc_spec("[email protected]-test +json+native+treesitter%[email protected]") == {
"pkg_name": "emacs",
"pkg_version": "29.2",
"pkg_version": "29.2-test",
"pkg_variants": '{"json": true, "native": true, "treesitter": true}',
"pkg_variants_dict": {"json": True, "native": True, "treesitter": True},
"compiler_name": "gcc",
Expand Down
3 changes: 2 additions & 1 deletion gantry/util/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ def parse_alloc_spec(spec: str) -> dict:
"""

# example: [email protected] +json+native+treesitter%[email protected]
spec_pattern = re.compile(r"(.+?)@([\d.]+)\s+(.+?)%([\w-]+)@([\d.]+)")
# this regex accommodates versions made up of any non-space characters
spec_pattern = re.compile(r"(.+?)@(\S+)\s+(.+?)%([\w-]+)@(\S+)")

match = spec_pattern.match(spec)
if not match:
Expand Down

0 comments on commit 32e451c

Please sign in to comment.