Use rockets python client #587
Merged
+2,277
−1,333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tribal-tec
force-pushed
the
python_using_rockets
branch
from
ca51f8d to
3abd3c4
Compare
rolandjitsu
reviewed
Oct 12, 2018
| from .api_generator import build_api | ||
| from .base import BaseClient | ||
| from .utils import obtain_registry, convert_snapshot_response_to_PIL, SCHEMA_ENDPOINT | ||
| from . import utils |
There was a problem hiding this comment.
Why not move what you have at . in utils?
There was a problem hiding this comment.
Actually I see that it is there, so why import that one differently? Why not:
from .utils import obtain_registry, convert_snapshot_response_to_PIL, SCHEMA_ENDPOINT, in_notebookThere was a problem hiding this comment.
That's a trap! The mocking otherwise does not work if the method is not used via its module name.
rolandjitsu
previously approved these changes
Oct 12, 2018
rdumusc
reviewed
Oct 12, 2018
brayns/common/types.h
Outdated
| { | ||
| SYNC, | ||
| ASYNC | ||
| } type{SYNC}; |
There was a problem hiding this comment.
A few things here:
- I feel that SYNC/ASYNC is somewhat confusing with the way the methods can be used be the client (a SYNC request can still be called in an async fashion by the async client)... What it really tries to say is "standard rpc" / "rpc that can be cancelled with progress report"; although I don't know how to express that succinctly...
- any reason to use
enuminstead ofenum class? - style: I'm not a big fan of this compact notation that's doing two things at once, it's easy to miss the class member hiding here, I'd rather write this in two lines, but that's personal.
brayns/common/types.h
Outdated
| }; | ||
|
|
||
| /** Description for RPC with one parameter. */ | ||
| struct RpcParameterDescription | ||
| struct RpcParameterDescription : public RpcDescription |
python/brayns/base.py
Outdated
| self.version.patch]) | ||
| return "Brayns version {0} running on {1}".format(version, self.http_url) | ||
|
|
||
| def set_colormap(self, palette, intensity=1, opacity=1, data_range=(0, 256)): |
There was a problem hiding this comment.
0,256 -> 0,255? Or is the range interpreted as [0,256[ (not included) ?
python/brayns/base.py
Outdated
| def function_builder(): | ||
| """Wrapper for returning the animation_slider() function.""" | ||
| def animation_slider(): | ||
| """.Show slider to control animation""" |
python/brayns/client.py
Outdated
| schemas)) | ||
| # pylint: enable=protected-access,cell-var-from-loop | ||
| schemas_dict[request.params['endpoint']] = response[0].result | ||
| build_api(self, registry, schemas_dict) |
There was a problem hiding this comment.
code duplicated from AsyncClient? Move to base class?
tribal-tec
force-pushed
the
python_using_rockets
branch
from
3abd3c4 to
77cd02d
Compare
- add new AsyncClient - improve code generation - add more tests
tribal-tec
force-pushed
the
python_using_rockets
branch
from
77cd02d to
cad051e
Compare
|
retest this please |
|
Need a review again actually |
rolandjitsu
approved these changes
Oct 15, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.