Closed limanjun99 closed 1 year ago
Overall LGTM. Some concerns:
- As shown in the image below the value of b is not displayed even when the execution was finished (Please look into this bug before approval)
- The wrong execution message is displayed when there are too many executions due to time limit exceeded (Please raise an issue for this)
The cause for issue 1 has been identified, raised an issue for it (and a related issue as well).
Issue 2 will be fixed in a future PR.
This PR upgrades the executor service so that it doesn't crash as much (e.g. when handling code that cannot be serialised to JSON). Tested with dev environment (https://feat-backend-improvement.d1fr5et3wgts3j.amplifyapp.com/).
Done:
Added black as python code formatter
Executor can read stdin and write to stdout
All data should be serialisable now, either because they were originally serialisable (e.g. int, float, bool, str), by custom handling (e.g. dict, list, set, tuple), or simply by returning the object's str() value.
Upgrade frontend to take in new response format
Simple testcases for executor service
Added error handling for syntax errors and runtime errors
Update field "ok" to "executed" (response format from executor), to show whether the code was executed (either to completion or runtime error) or not (e.g. syntax errors, parameter errors etc.)
Document the behaviour of the executor service so that we don't have to read the testcases to figure it out
Add limits to JSON data sent back (to prevent executor lambda sending huge responses and using up all network outgress)
Future Work: