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

Add support for CLI generation #829

Merged
merged 4 commits into from
May 10, 2024
Merged

Add support for CLI generation #829

merged 4 commits into from
May 10, 2024

Conversation

mayoor
Copy link
Member

@mayoor mayoor commented May 10, 2024

  • Added support for "ads aqua model register" CLI generation
  • Fixes to list imported models from user tenancy

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label May 10, 2024
if compartment_id:
model.with_compartment_id(compartment_id)
if project_id:
model.with_project_id(project_id)
Copy link
Member

Choose a reason for hiding this comment

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

this value is overwritten by COMPARTMENT_OCID and PROJECT_OCID on line 943, 944. I had added those two lines in a prev PR as it was failing create() method if those two attributes were not set.

We should do model.with_compartment_id(compartment_id or COMPARTMENT_OCID) and model.with_project_id(project_id or PROJECT_OCID) either here or below?

Copy link
Member

Choose a reason for hiding this comment

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

Probably there is a bug somewhere in the implementation, in case of compartment equal None it should be auto extracted form the env vars. Need to check this. For now we can use approach suggested by Vipul.

CLI builder from API interface. To be used with the DataClass only.
"""

def build_cli(self) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

just another yet approach

args = [
        f"--{field.name} {getattr(self, field.name)}"
        for field in fields(self.__class__)
        if getattr(self, field.name)
    ]
    return f"ads aqua {self._command} {' '.join(args)}"

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to for comprehension

raise HTTPError(400, Errors.NO_INPUT_DATA)

validate_function_parameters(data_class=CLIDetails, input_data=input_data)
command_details = CLIDetails(**input_data)
Copy link
Member

Choose a reason for hiding this comment

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

NIT: This potentially can break the code, if the input data contains some extra fields? In DataClassSerializable we support method - from_dict which has ignore_unknown attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

validate_function_parameters function just above that line will validate and throw error

validate_function_parameters(data_class=CLIDetails, input_data=input_data)
command_details = CLIDetails(**input_data)

interface = AquaCLIHandler.command_interface_map[
Copy link
Member

Choose a reason for hiding this comment

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

NIT: wouldn't it be better to use .get() and throw some error or return UNKNOW in case if the command not in map?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted the line to fail. The exception will get caught inside the handle_exception decorator and return invalid data.

fix compartment override issue
@mayoor mayoor merged commit 191bac7 into feature/aquav1.0.2 May 10, 2024
1 check passed
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

3 participants