Removes usage of overloaded STDERR interface #41
Conversation
* Clears up non-standard usage of STDERR * Remove legacy error handling logic * Now using STDIO 3 for JSON messages * Adds option for service.jail argument
* Used to test error conditions and behaviors * Currently only testing errors with JS services * Should cover most code paths for all langauges
* Adds tests for chaining services * Covers basic API and usage
* Makes service homedir configurable * Send 500 status code for timeout errors * Remove legacy code for checking NPM registry * Fix small regression in vm.stdout handler * All tests passing
* Better API semantic for users * Adds better code comments
* Replaces with FD3 comm channel * Compatible with new microcule APIs
* User functions should not require client lib * Uses script injection to wrap requests
bin/binaries/micro-python
Outdated
| @@ -16,6 +16,9 @@ def __prepare(): | |||
| __prepare() | |||
| del __prepare | |||
|
|
|||
| # inject microcule WSGI handler code | |||
| code = code + '\nimport microcule\nmicrocule.wsgi(Hook).run(app)\n' | |||
There was a problem hiding this comment.
This will break all "old-style" python hooks.
There was a problem hiding this comment.
Good catch, that is correct. I was consider the best ways to fix this.
I think we may have to create a separate code path / type for WSGI enabled applications. This way, users could spawn python functions in either the original way ( write directly to STDOUT ) or the new way of using WSGI style interfaces. We would only run this code injection line if we knew the user-code was defining WSGI style app interface.
I would be open to entirely removing the old python way, but I think we may have some backwards compatibility issues, so it would be worth supporting both.
There was a problem hiding this comment.
Why not leave as is? Tail construction if __name__…is standard in python world.
There was a problem hiding this comment.
The issue I had was that by leaving it, all user-defined functions now much import and call microcule python module. That breaks compatibility with some of the python WSGI servers I've seen.
Wouldn't it be better to have user-defined python functions work across multiple WSGI providers?
There was a problem hiding this comment.
This is limitation of single-file concept. In most cases WSGI-server is adapter between server-specific API (CGI, Apache module system, lighttpd plugin system) and WSGI compatible app. Python module microcule falls into same category. In other cases we can split WSGI-application object and WSGI wrapper to different files (sometimes it's just config), but not in our case.
We either should separate types of old-style and wsgi python hooks or provide method of wrapping as a module.
|
This change will break compatibility with all "old-style" python hooks. |
* Was breaking backwards compatibility * Seems more pythonic to not inject code
* Simplifies API to use only View * Default view is now just Mustache replacements * No longer using server-side DOM * Could be refactored into generic middleware
* Ends request when schema invalid * Actually sending JSON back
* Only partial coverage of plugins * Needs additional test cases
* Conforms to new API semantics * Refactor already completed * Only variable name changes
* Removes errant comment * Update .gitignore with nyc folders
* Ending response immediately for VM error * Do not attempt to run further middlewares
* Only for Travis * Will work locally
The original decision to use STDERR to pass JSON messages from child processes to the parent process has been somewhat problematic.
It was working well in previous cases before we added support for arbitrary binaries. Main issue is that it's valid for developers to use
STDERRto display non-critical logging messages. We were previously treating any non-json output fromSTDERRas an actual error to be piped directly back to the client. This isn't ideal, asSTDERRmessages should always be sent to a separate logging endpoint. Related: #34The breaking point for this came during testing of a new patch for hook.io which needs to call a well-known linux binary that consistently writes debugging information to
STDERR.This PR includes changes which move all JSON communication from child to parent process onto File Descriptor 3 ( which I believe is OK to do ).
@pyhedgehog - Could you please review the changes which were made to
pythonandpython-wsgisupport? I think I've pretty much got it working again, but we may want to change some of the API semantics.