Automatically select latest version of conda pack #1263
Conversation
prasankh
requested review from
darenr,
mayoor,
mrDzurb,
VipulMascarenhas,
qiuosier and
ahosler
as code owners
oracle-contributor-agreement
bot
added
the
OCA Verified
prasankh
changed the title
| get the latest conda pack. | ||
| """ | ||
| try: | ||
| auth = ads.auth.default_signer() |
There was a problem hiding this comment.
Is this the way we handle throughout? Is there an option to BYO auth through storage_options or equivalent?
There was a problem hiding this comment.
Reading other datasets from object storage works the same way.
There was a problem hiding this comment.
Let's move this logic out of "init" and into runtime. Maybe we could offer a tag "latest" that forces this logic?
The yaml file could set conda to "forecast_p311...latest", which triggers this method, and ultimately gives "forecast_p311..._v10" at runtime
| return tuple(map(int, match.group(1).split("."))) if match else (0,) | ||
|
|
||
| latest_obj = max(objects, key=lambda obj: extract_version(obj.name)) | ||
| return latest_obj.name.split("/")[-1] |
There was a problem hiding this comment.
Nice logic here 👍
There could be edge cases if any non-numerical versions are allowed, but we'll have to double check we don't support that.
We will still need to provide the python version and architecture by default, but this will greatly improve the current UX.
There was a problem hiding this comment.
This triggers when a user runs the operator init command, where we only support numerical versions.
Yes, we do need to support Python specific versions. At present, the latest releases are available only for Python 3.11.
There was a problem hiding this comment.
What should be the plan when we eventually release forecast_312_... ? Should we then update this logic? Or have some sort of transition period where we have both versions?
thanks
| return latest_obj.name.split("/")[-1] | ||
| except Exception as e: | ||
| logger.warning(f"Error while fetching latest conda pack: {e}") | ||
| return base_conda |
There was a problem hiding this comment.
Is base_conda a valid pack?
Isn't it just forecast_py310_x86-84_v ?
There was a problem hiding this comment.
Base conda is coming from MLOperator and it's forecast_p311_cpu_x86_64_v10
| @@ -117,6 +123,29 @@ def info( | |||
| ) | |||
|
|
|||
|
|
|||
| def get_latest_conda_pack(prefix, base_conda) -> str: | |||
| """ | |||
| get the latest conda pack. | |||
There was a problem hiding this comment.
can you please clarify what prefix and base_conda should be?
And is it guaranteed to return a valid conda path? or slug?
There was a problem hiding this comment.
base_conda and prefix is coming from MLoperator, If get_latest_conda_pack fails to get the latest conda pack, it returns base_conda which is also valid.
| @@ -157,6 +186,7 @@ def init( | |||
|
|
|||
| # load operator info | |||
| operator_info: OperatorInfo = OperatorLoader.from_uri(uri=type).load() | |||
| operator_info.conda = get_latest_conda_pack(operator_info.prefix, operator_info.conda) | |||
There was a problem hiding this comment.
Isn't the prefix service_pack/cpu/AI_Forecasting_Operator/ ?
So how does the method know that it's looking for the latest "forecast" pack?
There was a problem hiding this comment.
Yes the prefix is service_pack/cpu/AI_Forecasting_Operator/ . The method list the bucket with this prefix and it always picks the latest when the user runs the init command with the fallback to base_conda, both of these are coming from MLOperator file
There was a problem hiding this comment.
For other operators, we can specify a prefix for that operator to list in MLOperator file in the bucket and it will retrieve the latest item after that prefix. Prefix can be different for different operators.
No description provided.