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

Create new blog post. #45

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BonfaceKilz
Copy link
Contributor

No description provided.

Copy link
Contributor

@Alexanderlacuna Alexanderlacuna left a comment

Choose a reason for hiding this comment

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

Minor : Since this is a blog, could you add a link or provide context for what 'virtuoso' as part of the introduction.

Copy link
Contributor

@Alexanderlacuna Alexanderlacuna left a comment

Choose a reason for hiding this comment

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

Question: Are the blogs meant as internal tools for GeneNetwork, or can we include them as part of the GeneNetwork blogs? See: an example: cd.genenetwork.org/gn-docs/blogs.

@BonfaceKilz
Copy link
Contributor Author

Question: Are the blogs meant as internal tools for GeneNetwork, or can we include them as part of the GeneNetwork blogs? See: an example: cd.genenetwork.org/gn-docs/blogs.

Great point, thanks for bringing this up! Initially, I went with this approach because it was the simplest solution at the time. @pjotrp @robwwilliams, when you have a moment, I’d love to hear your thoughts on this. Since we have our engineering blog entries in gn-gemtext, do you think it makes sense to migrate them over to our main website? Personally, I'd keep it in gn-gemtext, as it is engineering related. And leave our main website for things related to bio-informatics. But since you bring this up, it's a nice opportunity to add more bio-informatics entries related to GN from folk like @fetche-lab

Signed-off-by: Munyoki Kilyungi <[email protected]>
@@ -0,0 +1,68 @@
# Running Background Processes in Python using Popen
Copy link
Contributor

Choose a reason for hiding this comment

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

Header seems misleading. I think it's just that virtuoso uses the flags +foreground to state that the process runs in the foreground. Here's what think these mean:

  • foreground: you can actively interact with a process in your terminal. If you press Ctrl-Z it's taken to the backgound. Typing fg brings it back to the background
  • Background: You cannot interact with the process, but it's running/paused in the background. Typing bg actually runs the process in your background.
  • Daemon: Process ran but detached itself from it's parent and attached itself to P1D 1 (init) hence when we close the terminal, this process won't end.


* author: bonfacem
* reviewed-by: jnduli

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we restructure this document? I'd rather we provide the solution first and historical context later, because future consumers will most likely want to solve their problem first.

19 psutil.Process(pid_).kill()
```

Using "subprocess.Popen"(L3-L13), we spawn the Virtuoso process in the background. However, after the tests finished, the Virtuoso process remained running in the background, leading to failures in subsequent tests. To handle this, we had to manually kill it(L18-L19). This approach is clanky because we expected the Virtuoso process to terminate when the parent process ended. Calling "pid.terminate()" didn’t work since Virtuoso was spawned under a different PID without a parent.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the virtuoso process starts in the background, but rather is daemonized. This means once the tests finish, the virtuoso process can't be killed because it isn't a child of pytest.

Also, it isn't that virtuoso was spawned under a different PID. Here's a video explaining the process: https://youtu.be/hgb-1f6BlGI?list=PLSIUOFhnxEiC3YTdxwqZqgEY5imVL8U8J&t=1120

```
1 command = f"virtuoso-t +foreground +wait +no-checkpoint +configfile {init_file}"
2 with subprocess.Popen(
3 command,
Copy link
Contributor

Choose a reason for hiding this comment

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

let's explain why this is a string and the previous command is an array. This took me a long time to figure out btw :D

1 command = f"virtuoso-t +foreground +wait +no-checkpoint +configfile {init_file}"
2 with subprocess.Popen(
3 command,
4 shell=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

let's explain why shell=True. I think its because virtuoso won't output anything if it determines it isn't running in a tty.

19 pid.terminate()
```

In this version, by running Virtuoso in a shell, we can use the "+foreground" option(L1), allowing the process to run in a non-blocking way interactively. This approach also simplifies termination by way of "pid.terminate()"(L19) instead of hunting for PIDs.
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. question: what does non-blocking way mean?
  2. Can we instead mention that pid.terminate() closes the process. Mentioning hunting of PIDs is part of the history/context for how you ended up with this situation, so perhaps should be in the previous section where you explain why you had to hunt for these.

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.

3 participants