[ODSC-69670] AQUA version visibility changes #1215
Conversation
kumar-shivam-ranjan
requested review from
darenr,
mayoor,
mrDzurb,
VipulMascarenhas,
qiuosier and
ahosler
as code owners
oracle-contributor-agreement
bot
added
the
OCA Verified
|
📌 Cov diff with main: 📌 Overall coverage: |
ads/aqua/version.json
Outdated
| @@ -0,0 +1,6 @@ | |||
| { | |||
| "installed": { | |||
There was a problem hiding this comment.
I think ADS version we can get form the ads config. Otherwise will heave to update both places when we release ADS
There was a problem hiding this comment.
Updated. Thanks!
ads/aqua/version.json
Outdated
| { | ||
| "installed": { | ||
| "ads": "2.14.2", | ||
| "aqua": "0.1.3.0" |
There was a problem hiding this comment.
wouldn't it be better to keep the aqua version details in ads/aqua/__init__.py instead of this external json? We can then get both the versions like:
from ads import __version__ as ads_version
from ads.aqua import __version__ as aqua_version
There was a problem hiding this comment.
The ADS version is already stored in the toml file, no need to move it to the init file.
class ADSVersionHandler(AquaAPIhandler):
"""The handler to get the current version of the ADS."""
@handle_exceptions
def get(self):
self.finish({"data": metadata.version("oracle_ads")})
Regarding storing the version in init.py, I’d still prefer to keep it in a version.json file. Since init is no longer required and will likely be phased out soon, relying on it for versioning may not be ideal.
Maintaining the version in a dedicated version.json file is also easier and more visible, making it less likely to be overlooked during updates. Once AQUA becomes an independent project, we can move the version into a pyproject.toml file.
There was a problem hiding this comment.
Updated. Thanks!
|
📌 Cov diff with main: 📌 Overall coverage: |
|
|
||
| """ | ||
|
|
||
| current_aqua_version_path = os.path.join( |
There was a problem hiding this comment.
Let's make it a bit reliable, let's wrap the getting latest_version_artifact_path with the try catch block? Otherwise we don't want to break entire AQUA because of this validation :)
There was a problem hiding this comment.
I was thinking of adding this try catch block at UI side so that we don't break entire AQUA but yeah this also sounds good. will update. Thanks
There was a problem hiding this comment.
Updated , thanks
|
📌 Cov diff with main: 📌 Overall coverage: |
…oracle/accelerated-data-science into feature/aqua_version_visibility
|
📌 Cov diff with main: 📌 Overall coverage: |
Description
This PR is intended to enhance version visibility for AQUA. Users will get notification to deactivate/activate their notebooks whenever new version of AQUA is available. The latest deployed version of AQUA is fetched from conda pack OSS bucket.
Related PRs
Unit tests
Test results