Support serving multiple libraries by one server #47
Support serving multiple libraries by one server #47robooo wants to merge 4 commits intorobotframework:masterfrom
Conversation
Changes has been made to support serving a list of libraries: robotframework#19
serving multiple classes by one single server
Changes has been made to support serving a list of libraries: robotframework#19
update example for change of serving multiple libraries
|
We are using this adaptions now for more than one year. It is absolut stable and we recognized no problems. Is there a possibility that this feature will be merged into master in the near future? And if not, are there any doubts about this integration? Thanks in advance, |
srinivaschavan
left a comment
There was a problem hiding this comment.
@robooo , these code changes look good. I ended up making similar changes before looking at your PR.
For completeness sake, I think the docs should be updated to indicate that library should be a list. Also, I feel that we should keep backward compatibility support for library not being a list.
|
@srinivaschavan @robooo @loumaran would be awesome to see this merged in. Can we get an update here? @srinivaschavan I'm happy to update documentation if that's what's needed to move this along. That would be primarily the readme in this repo? Perhaps adding a |
|
@Erich-McMillan , yes updating the README should suffice. However, the real concern with this PR is that it is not backwards compatible. IMO, we should handle the case where users have not specified a list of libraries. |
Agreed, best to allow users to continue passing in a single library without putting it into a list. Would the best path forward be to have the constructor check the type of Also if no feedback from previous authors, I can create a new PR to introduce those changes alongside docs. |
| ``stop_remote_server`` XML-RPC method. | ||
| """ | ||
| self._library = RemoteLibraryFactory(library) | ||
| self._library = [RemoteLibraryFactory(library_) |
There was a problem hiding this comment.
@srinivaschavan to maintain backward compatibility could this be a solution?
| self._library = [RemoteLibraryFactory(library_) | |
| if isinstance(libraries, list): | |
| self._libraries = [RemoteLibraryFactory(library_) | |
| for library_ in libraries] | |
| else: | |
| self._libraries = [RemoteLibraryFactory(libraries)] |
Though not sure if checking isinstance is the most pythonic, maybe checking to see if the item supports __iter__ would be more robust?
There was a problem hiding this comment.
I think this solutions works. Best if you create a new PR with these changes.
No description provided.