Skip to content

Conversation

@Meldiron
Copy link
Contributor

@Meldiron Meldiron commented Jan 13, 2023

Adds support for communication with both v2 and v3 of Open Runtime.

@Meldiron Meldiron marked this pull request as ready for review February 2, 2023 18:11
->param('runtimeId', '', new Text(64), 'The runtimeID to execute.')
->param('payload', '', new Text(8192), 'Data to be forwarded to the function, this is user specified.', true)
->param('variables', [], new Assoc(), 'Environment variables passed into runtime.', true)
->param('body', '', new Text(20971520), 'Data to be forwarded to the function, this is user specified.', true)
Copy link
Contributor

Choose a reason for hiding this comment

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

on what basis is this limit set? what if the payload exceeds this size? For example when uploading a file in chunks ? Can we make sure it accepts 5MB ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should support up to 20MB, which is limit that we consider reasonable even in Appwrite.
5MB chunk uploads should work just fine. Trying to upload 30MB file at once will fail.

],
[
'image' => 'openruntimes/node:v2-18.0',
'image' => 'openruntimes/node:v3-18.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we have both v3 and v2 images, does the warm up task in the beginning also pull the v3 images?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only v3 images. v2 are no longer pulled.

Copy link
Contributor

@christyjacob4 christyjacob4 left a comment

Choose a reason for hiding this comment

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

Looks good. Please address the comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants