Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed-loading large libraries with get_library_information #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JFoederer
Copy link
Contributor

Hi @pekkaklarck,

Could you please review these changes and provide feedback on this pull request in combination with pull requests
Python remote server Test suite update and Implement get_library_information interface for Remote libraries (Robot side)?

Together they make a long overdue step for the Python remote server, fixing a number of issues

Python remote server issues:

Robot framework issue:

There is however some dotting the ï-s to do and some decisions to be made, for which I would value your vision.
for instance:

  • How to version this update (and update docu accordingly)
  • Python 2 support (Not compatible in the current state)
  • How to deal with binary vs unicode (and fix tests accordingly)

…ibraries

Include support for the get_library_information API extention.

Also includes a fix to deal with kwonly arguments and an extension to allow hosting multiple libraries on a single server.
@pekkaklarck
Copy link
Member

pekkaklarck commented Mar 4, 2021

Sorry for neglecting this PR for so long. Been busy with getting RF 4.0 work and RoboCon planning. I won't have time for this project until RF 4.0 final is out, but I will look at the related issue on the RF side to see could we still get those changes into that release.

@JFoederer
Copy link
Contributor Author

Hey Pekka,
In the meantime I also joined the Slack community, where I got a bit more feel for what is going on overall with the RF developments. From that it was clear the attention had to go elsewhere for a bit, so no problem.

Getting the Robot side preparations for these improvements integrated already would certainly ease the delivery of this update later on. Let's see, let me know if you have questions!

Explicitly add __intro__ and __init__ docu to get_library_information()

Also includes a fix. Not all libraries have the _names attribute (e.g. Reserved).
@pekkaklarck
Copy link
Member

RoboCon sprints are running today and we just decided to talk about Python remote servers there at 12:00 UTC. Sprints are organized on Gather and you can access them via https://venue.robocon.io. This is very late notice but it would be great if you could make it there @JFoederer!

@pekkaklarck
Copy link
Member

A problem with this PR is that it contains so many different fixes and enhancements that reviewing everything is really hard. If you @JFoederer are still interested on this, could you create smaller PRs for each enhancement?

Right now I'm planning to create v1.1.1 with urgent Python 3.10 and 3.11 compatibility fixes, but I hope I could return to this project soon again and we could then do v1.2 with some new functionality. We can discuss this more on Slack as well.

@pekkaklarck
Copy link
Member

I believe the most important enhancement would be adding get_library_information. There's also PR #79 adding it, but it has some problems as well.

@JFoederer
Copy link
Contributor Author

Ah yes, this PR. The real struggle was to get a decent base to start from. There were tests failing, test missing and several things not working properly, if I recall correctly. I didn't have the overview at the time to isolate improvements. I was just happy to get something working in my context then.

The context being the option to host multiple libraries. Or actually hosting remote Robot runs for system-of-systems testing. Getting that to work made me fall from one thing into the other, including massive load times, until I had it working.

I am not sure I'll have time for this on short notice, but I'd be happy to help out where I can.

@pekkaklarck
Copy link
Member

I'll look at this again when starting v1.2 development. I'll now create v1.1.1 with just Python compatibility fixes.

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.

2 participants