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

Azure Container Apps session execute new api-version #8249

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/containerapp/HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ upcoming
* 'az containerapp create': Fix Role assignment error when the default Azure Container Registry could not be found
* Upgrade api-version to 2024-10-02-preview
* 'az containerapp create/update': `--yaml` support property pollingInterval and cooldownPeriod
* 'az containerapp session code-interpreter': `--path` support upload-file/list-files/show-file-content/show-file-metadata/delete-file for code interpreter sessions
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @bowen5 @yitaopan
About the request payload change and the response breaking change, we should update the description about them

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 'az containerapp session code-interpreter': `--path` support upload-file/list-files/show-file-content/show-file-metadata/delete-file for code interpreter sessions
* 'az containerapp session code-interpreter upload-file/list-files/show-file-content/show-file-metadata/delete-file': Support `--path` to specify the path of code interpreter session

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed


1.0.0b4
++++++
Expand Down
27 changes: 15 additions & 12 deletions src/containerapp/azext_containerapp/_clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
logger = get_logger(__name__)

PREVIEW_API_VERSION = "2024-10-02-preview"
SESSION_API_VERSION = "2024-02-02-preview"
POLLING_TIMEOUT = 1500 # how many seconds before exiting
POLLING_SECONDS = 2 # how many seconds between requests
POLLING_TIMEOUT_FOR_MANAGED_CERTIFICATE = 1500 # how many seconds before exiting
Expand Down Expand Up @@ -1018,7 +1017,7 @@ def list(cls, cmd, resource_group_name, environment_name):


class SessionPoolPreviewClient():
api_version = SESSION_API_VERSION
api_version = PREVIEW_API_VERSION
Copy link
Contributor

@Greedygre Greedygre Nov 11, 2024

Choose a reason for hiding this comment

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

I will ask azure cli folks help to merge this PR: #8223

It will have some conflict with your PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR has been merge. Please resolve the conflict.

Copy link
Member Author

Choose a reason for hiding this comment

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

Merged and conflicts resolved.


@classmethod
def create(cls, cmd, resource_group_name, name, session_pool_envelope, no_wait=False):
Expand Down Expand Up @@ -1157,11 +1156,11 @@ def list_by_subscription(cls, cmd, formatter=lambda x: x):


class SessionCodeInterpreterPreviewClient():
api_version = SESSION_API_VERSION
api_version = PREVIEW_API_VERSION

@classmethod
def execute(cls, cmd, identifier, code_interpreter_envelope, session_pool_endpoint, no_wait=False):
url_fmt = "{}/code/execute?identifier={}&api-version={}"
url_fmt = "{}/executions?identifier={}&api-version={}"
request_url = url_fmt.format(
session_pool_endpoint,
identifier,
Expand All @@ -1180,10 +1179,11 @@ def execute(cls, cmd, identifier, code_interpreter_envelope, session_pool_endpoi
return r.json()

@classmethod
def upload(cls, cmd, identifier, filepath, session_pool_endpoint, no_wait=False):
url_fmt = "{}/files/upload?identifier={}&api-version={}"
def upload(cls, cmd, identifier, filepath, path, session_pool_endpoint, no_wait=False):
url_fmt = "{}/files?{}identifier={}&api-version={}"
request_url = url_fmt.format(
session_pool_endpoint,
"path=" + path + "&" if path is not None else "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"path=" + path + "&" if path is not None else "",
f"path={path}&" if path is not None else "",

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

identifier,
cls.api_version)

Expand Down Expand Up @@ -1220,11 +1220,12 @@ def upload(cls, cmd, identifier, filepath, session_pool_endpoint, no_wait=False)
return r.json()

@classmethod
def show_file_content(cls, cmd, identifier, filename, session_pool_endpoint):
url_fmt = "{}/files/content/{}?identifier={}&api-version={}"
def show_file_content(cls, cmd, identifier, filename, path, session_pool_endpoint):
url_fmt = "{}/files/{}/content?{}identifier={}&api-version={}"

This comment was marked as resolved.

This comment was marked as resolved.

request_url = url_fmt.format(
session_pool_endpoint,
filename,
"path=" + path + "&" if path is not None else "",
Copy link
Contributor

Choose a reason for hiding this comment

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

same as L1186

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed

identifier,
cls.api_version)

Expand All @@ -1235,11 +1236,12 @@ def show_file_content(cls, cmd, identifier, filename, session_pool_endpoint):
return json.dumps(r.content.decode())

@classmethod
def show_file_metadata(cls, cmd, identifier, filename, session_pool_endpoint):
url_fmt = "{}/files/{}?identifier={}&api-version={}"
def show_file_metadata(cls, cmd, identifier, filename, path, session_pool_endpoint):
url_fmt = "{}/files/{}?{}identifier={}&api-version={}"
request_url = url_fmt.format(
session_pool_endpoint,
filename,
"path=" + path + "&" if path is not None else "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"path=" + path + "&" if path is not None else "",
f"path={path}&" if path is not None else "",

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

identifier,
cls.api_version)
logger.warning(request_url)
Expand All @@ -1248,11 +1250,12 @@ def show_file_metadata(cls, cmd, identifier, filename, session_pool_endpoint):
return r.json()

@classmethod
def delete_file(cls, cmd, identifier, filename, session_pool_endpoint, no_wait=False):
url_fmt = "{}/files/{}?identifier={}&api-version={}"
def delete_file(cls, cmd, identifier, filename, path, session_pool_endpoint, no_wait=False):
url_fmt = "{}/files/{}?{}identifier={}&api-version={}"
request_url = url_fmt.format(
session_pool_endpoint,
filename,
"path=" + path + "&" if path is not None else "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"path=" + path + "&" if path is not None else "",
f"path={path}&" if path is not None else "",

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

identifier,
cls.api_version)

Expand Down
11 changes: 4 additions & 7 deletions src/containerapp/azext_containerapp/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,10 @@
}

SessionCodeInterpreterExecution = {
"properties": {
"identifier": None,
"codeInputType": None,
"executionType": None,
"code": None,
"timeoutInSeconds": None
}
"codeInputType": None,
Copy link
Contributor

Choose a reason for hiding this comment

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

with one property identifier removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The backend Api is switched to use 2024-10-02-preview in this PR, in which the identifier property is removed in the payload, but still exists in the query parameter.

"executionType": None,
"code": None,
"timeoutInSeconds": None
}

DaprComponentResiliency = {
Expand Down
2 changes: 1 addition & 1 deletion src/containerapp/azext_containerapp/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ def load_arguments(self, _):
with self.argument_context('containerapp session code-interpreter', arg_group='file') as c:
c.argument('filename', help="The file to delete or show from the session")
c.argument('filepath', help="The local path to the file to upload to the session")
c.argument('path', help="The path to list files from the session")
c.argument('path', help="The path of the file from the session")
Copy link

Choose a reason for hiding this comment

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

How about The path of files in the session. from/to terms are used w/ verbs, like above upload to or delete from.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed


with self.argument_context('containerapp session code-interpreter', arg_group='execute') as c:
c.argument('code', help="The code to execute in the code interpreter session")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,14 @@ def validate_arguments(self):
"sessionPools")

def construct_payload(self):
self.session_code_interpreter_def["properties"]["identifier"] = self.get_argument_identifier()

self.session_code_interpreter_def["properties"]["codeInputType"] = "inline"
self.session_code_interpreter_def["properties"]["executionType"] = "synchronous"
self.session_code_interpreter_def["codeInputType"] = "inline"
self.session_code_interpreter_def["executionType"] = "synchronous"

if self.get_argument_timeout_in_seconds() is not None:
self.session_code_interpreter_def["properties"]["timeoutInSeconds"] = self.get_argument_timeout_in_seconds()
self.session_code_interpreter_def["timeoutInSeconds"] = self.get_argument_timeout_in_seconds()
else:
self.session_code_interpreter_def["properties"]["timeoutInSeconds"] = 60
self.session_code_interpreter_def["properties"]["code"] = self.get_argument_code()
self.session_code_interpreter_def["timeoutInSeconds"] = 60
Copy link
Contributor

Choose a reason for hiding this comment

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

The self.get_argument_timeout_in_seconds() will never be None because there is a default value:

   --timeout-in-seconds           : Duration in seconds code in session can run prior to timing out
                                     0 - 60 secs, e.g. 30.  Default: 60.

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense, fixed.

self.session_code_interpreter_def["code"] = self.get_argument_code()

def execute(self):
try:
Expand All @@ -111,6 +109,7 @@ def upload(self):
cmd=self.cmd,
identifier=self.get_argument_identifier(),
filepath=self.get_argument_filepath(),
path=self.get_argument_path(),
session_pool_endpoint=self.get_sessionpool_endpoint(),
no_wait=self.get_argument_no_wait())
except Exception as e:
Expand All @@ -122,6 +121,7 @@ def show_file_content(self):
cmd=self.cmd,
identifier=self.get_argument_identifier(),
filename=self.get_argument_filename(),
path=self.get_argument_path(),
session_pool_endpoint=self.get_sessionpool_endpoint())
except Exception as e:
handle_raw_exception(e)
Expand All @@ -132,6 +132,7 @@ def show_file_metadata(self):
cmd=self.cmd,
identifier=self.get_argument_identifier(),
filename=self.get_argument_filename(),
path=self.get_argument_path(),
session_pool_endpoint=self.get_sessionpool_endpoint())
except Exception as e:
handle_raw_exception(e)
Expand All @@ -152,6 +153,7 @@ def delete_file(self):
cmd=self.cmd,
identifier=self.get_argument_identifier(),
filename=self.get_argument_filename(),
path=self.get_argument_path(),
session_pool_endpoint=self.get_sessionpool_endpoint(),
no_wait=self.get_argument_no_wait())
except Exception as e:
Expand Down
4 changes: 4 additions & 0 deletions src/containerapp/azext_containerapp/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -2989,6 +2989,7 @@ def upload_session_code_interpreter(cmd,
resource_group_name,
identifier,
filepath,
path=None,
session_pool_location=None):
raw_parameters = locals()
session_code_interpreter_decorator = SessionCodeInterpreterCommandsPreviewDecorator(
Expand All @@ -3009,6 +3010,7 @@ def show_file_content_session_code_interpreter(cmd,
resource_group_name,
identifier,
filename,
path=None,
session_pool_location=None):
raw_parameters = locals()
session_code_interpreter_decorator = SessionCodeInterpreterCommandsPreviewDecorator(
Expand All @@ -3029,6 +3031,7 @@ def show_file_metadata_session_code_interpreter(cmd,
resource_group_name,
identifier,
filename,
path=None,
session_pool_location=None):
raw_parameters = locals()
session_code_interpreter_decorator = SessionCodeInterpreterCommandsPreviewDecorator(
Expand Down Expand Up @@ -3069,6 +3072,7 @@ def delete_file_session_code_interpreter(cmd,
resource_group_name,
identifier,
filename,
path=None,
session_pool_location=None):
raw_parameters = locals()
session_code_interpreter_decorator = SessionCodeInterpreterCommandsPreviewDecorator(
Expand Down
Loading
Loading