Vlm match endpoint #4506
Vlm match endpoint #4506
Conversation
|
|
||
| def _parse_match_query(query: dict) -> tuple[str, int, str, str, str]: | ||
| missing_params = [key for key in QUERY_PARAMS if key not in query] | ||
| if missing_params: |
There was a problem hiding this comment.
If there's not a huge rush on getting this in, now might be a good time to explore pydantic validation.
There was a problem hiding this comment.
Theres not a rush to get this specific code out, but I have other things on my plate that I think are higher priority for me. So its less about getting this code released and more about freeing up my time for other feature work
| spark_conf = {} | ||
| # memory limits adapted from https://github.com/hail-is/hail/blob/main/hail/python/hailtop/hailctl/dataproc/start.py#L321C17-L321C36 | ||
| if MACHINE_MEM: | ||
| spark_conf['spark.driver.memory'] = f'{int((int(MACHINE_MEM) - 11) * JVM_MEMORY_FRACTION)}g' |
There was a problem hiding this comment.
The code to build spark_conf here looks identical to how it's done in the hail_search web_app, can this be put in a helper function somewhere?
There was a problem hiding this comment.
So ideally yes. However, the docker builds for hail_search and the vlm each only include the single directory for their respective apps, and then the unit tests and releases are only triggered on change to those specific directories. Creating a shared directory that is included in the build for both apps and then updating the github triggers to include the additional shared directory is possible, but it seemed like more work than it was worth to avoid this relatively small amount of copy-paste. However, if you and/or @bpblanken feel like its worth doing than I can do so. However, in the interest of keeping scope sane I would recommend that this gets merged first and then I do the shared code move in a subsequent PR
There was a problem hiding this comment.
Considering the amount of work it would take to make this code shared, I think it's fine if it's duplicated.
| coverage run --source="./vlm" --omit="./vlm/__main__.py" -m pytest vlm/ | ||
| coverage report --fail-under=90 | ||
| coverage report --fail-under=95 |
Vlm multi-build match
Adds the main matc endpoint for the VLM
Note: this can not be merged until VLM is disabled in dev/ we get compliance sign off to release this publicly