Skip to content

Conversation

@richardmtaylor
Copy link
Contributor

No description provided.

@richardmtaylor richardmtaylor requested review from ivolz and jexh April 10, 2025 17:49
@ivolz
Copy link
Contributor

ivolz commented Apr 11, 2025

LGTM. One thing to note if it ever arises in a production app and we get complaints: the nature of isolates causes a timeout to happen in the isolate so handling of lack of networking has to be built in the isolate logic by the client application; the example I have been provided with crashed, the shapes out times out with ClientException with SocketException: Connection timed out (OS Error: Connection timed out, errno = 110), address = shapes.approov.io, port = 47630, uri=https://shapes.approov.io/v1/shapes.

Copy link
Contributor

@ivolz ivolz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jexh jexh left a comment

Choose a reason for hiding this comment

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

Minor fixes suggested. Otherwise LGTM

Copy link
Contributor

@jexh jexh left a comment

Choose a reason for hiding this comment

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

LGTM

@jexh jexh merged commit 9b0c9bd into main May 9, 2025
@jexh jexh deleted the feature/isolate-fix branch May 9, 2025 08:34
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