Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aqua v1.0.1 release #803

Merged
merged 91 commits into from
Apr 24, 2024
Merged

Aqua v1.0.1 release #803

merged 91 commits into from
Apr 24, 2024

Conversation

VipulMascarenhas
Copy link
Member

@VipulMascarenhas VipulMascarenhas commented Apr 23, 2024

Description

This PR releases v1.0.1 iteration of Aqua, mainly comprising of bug fixes and improving unit tests and covers the following:

  • Fixed bugs
  • Added compatibility check for GA region
  • additional telemetry
  • logging and error messages improvements

ADS version will be updated to v2.11.8.

Jira

ODSC-55689

VipulMascarenhas and others added 30 commits April 15, 2024 23:24
…finetune.

- added tracking shapes for evaluation
…finetune.

- added finetune shape, finetune model and combination of a shape and model
ads/aqua/deployment.py Outdated Show resolved Hide resolved
ads/aqua/deployment.py Outdated Show resolved Hide resolved
Copy link

📌 Cov diff with main:

Coverage-1%

📌 Overall coverage:

Coverage-21.35%

ads/aqua/base.py Outdated Show resolved Hide resolved

def __init__(
self,
log_level: str = os.environ.get(ENV_VAR_LOG_LEVEL, "ERROR").upper(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if user pass empty string as a log_level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ads aqua model list --log-level=""

just checked, it fails with the message:

  File "/Users/<user>/miniconda3/envs/ads-local-v38/lib/python3.8/logging/__init__.py", line 198, in _checkLevel
    raise ValueError("Unknown level: %r" % level)
ValueError: Unknown level: ''

@mingkang111 can we have an input check to fix this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just one line check, can you add from you side instead of creating another PR and waiting for review? Thanks!
Something like:

if not log_level in [...]:
    exit()

Copy link
Member Author

@VipulMascarenhas VipulMascarenhas Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, I'll update it my PR, might be better to default to Error level instead of exit. thanks Ming!

Copy link
Member

@mingkang111 mingkang111 Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, ads aqua --help provides what value can be put there. If user provide wrong value, it is should be ok to fail. And the error raised here also indicate the cause for the failure, i.e. the value provided is invalid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true, but in this case the error should be human friendly. I'm ok with any solution that would make it more clear for end user.

ads/aqua/cli.py Show resolved Hide resolved
ads/aqua/cli.py Show resolved Hide resolved
ads/aqua/decorator.py Show resolved Hide resolved
ads/aqua/deployment.py Outdated Show resolved Hide resolved
ads/aqua/deployment.py Outdated Show resolved Hide resolved
Copy link

📌 Cov diff with main:

Coverage-1%

📌 Overall coverage:

Coverage-21.35%

ads/aqua/cli.py Outdated Show resolved Hide resolved
Copy link

📌 Cov diff with main:

Coverage-2%

📌 Overall coverage:

Coverage-21.35%

Copy link

📌 Cov diff with main:

Coverage-97%

📌 Overall coverage:

Coverage-61.57%

@VipulMascarenhas VipulMascarenhas merged commit c8526cc into main Apr 24, 2024
14 checks passed
@VipulMascarenhas VipulMascarenhas deleted the feature/aquav1.0.1 branch April 24, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants