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

Version 0.0.1 #13

Merged
merged 87 commits into from
Apr 7, 2022
Merged

Version 0.0.1 #13

merged 87 commits into from
Apr 7, 2022

Conversation

GitHK
Copy link
Collaborator

@GitHK GitHK commented Mar 11, 2022

What do these changes do?

Add a base library to be used for control cases.
NOTE: this package was build using cookiecutter-hypermodern-python for convenience.

How does it work?

  • The osparc_control package provides an easy way to define a supported interface by providing a list of CommandManifest.
  • A CommandManifest is the interface(signature) of the supported command.
  • Each request from a remote is validated against CommandManifest. A CommandReceived confirmation is always send to each command. The requester expects this command to be delivered (this is required to avoid the requesting part being stuck awaiting forever if the remote part is not running). A 1 minute interval is defined to detect this situation.
  • It supports 3 types of commands defined by CommandType:
class CommandType(str, Enum):
    # the command expects no reply
    WITHOUT_REPLY = "WITHOUT_REPLY"

    # the command will provide a reply
    # the user is require to check for the results
    # of this reply
    WITH_DELAYED_REPLY = "WITH_DELAYED_REPLY"

    # the command will return the result immediately
    # and user code will be blocked until reply arrives
    # used for very fast replies (provide data which already exists)
    WITH_IMMEDIATE_REPLY = "WITH_IMMEDIATE_REPLY"

How to test

You require poetry on your system, please make sure it is installed.

In a virtualenv install dev dependencies

make install-dev

Run suite with python 3.6 and nox.

make nox-36

If you do not have python 3.6 on your system feel free to run with a different python interpreter by changing the below command (replace 3.6 with your version of python:

nox -p 3.6

Examples

Please follow the updated README.md

What changed

Related issues

@GitHK GitHK self-assigned this Mar 11, 2022
@GitHK GitHK requested review from pcrespov and sanderegg April 7, 2022 08:50
@GitHK GitHK added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 7, 2022
src/osparc_control/__main__.py Show resolved Hide resolved
tests/test_examples.py Outdated Show resolved Hide resolved
# run provided scripts
print(f"Running permutation {permutation}")
processes = [
Popen( # noqa: S607
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why dont' you use subprocess.run instead? the latest versions of python provide this more convenient helper that is far more readable and has preconfigured the most common setups. Using raw Popen can be sometimes errorprone ... (we already had this discussion several times... :-) )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not what I wanted or needed. From the docs

Run the command described by args. Wait for command to complete, then return a CompletedProcess instance.

I need to start 2 scripts and wait for them to complete.


# ensure all processes finished successfully
for process in processes:
stdout, _ = process.communicate()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but these do not run in parallel as you claim in assert_run_in_parallel , right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they do run in parallel, otherwise it would not be possible for example/2_base_time_add to finish.
In the above loop I wait for the process to provide a returncode, meaning it finished execution.

Here I fetch the standard output.

Copy link
Collaborator

@KZzizzle KZzizzle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! let's get this in!

@@ -1,5 +1,5 @@
[flake8]
select = B,B9,C,D,DAR,E,F,N,RST,S,W
select = B,B9,C,DAR,E,F,N,RST,S,W
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these entries are amazingly clear 🤣

Copy link
Collaborator

@elisabettai elisabettai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me! Looking forward to testing it.

@GitHK GitHK merged commit 6f10b57 into master Apr 7, 2022
@GitHK GitHK deleted the initial-version branch April 7, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants