-
Notifications
You must be signed in to change notification settings - Fork 80
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
Unmoi job completion #2029
Unmoi job completion #2029
Conversation
BTW: I deployed this in the test environment and I did not have any problem running existing jobs. |
Code looks great, still 2 errors. @wasade, @ElDeveloper, @tanaes, could you take a look? |
safe_submit(job.user.email, _job_completer, job_id, payload) | ||
p = Process(target=_job_completer, | ||
args=(job_id, self.request.body)) | ||
p.start() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this blocking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No
yes, yes, yes, yes, YEEEESSSSS!!!!! My brain was starting to hurt! So the source of the problem was these missing lines. Basically, if the global variable QIITA_CONFIG_FP was not set, it was not removed after setting it up in the setUp of the function. When the child process was spawn, it inherited the environment variables from the parent process, which was set up to a temporary file that was getting removed just after the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this is beyond me at the moment -- just a few questions to help me h detest and better!
|
||
# When Popen executes, the shell is not in interactive mode, | ||
# so it is not sourcing any of the bash configuration files | ||
# We need to source it so the env_script are available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this documentation! I think this has tripped me up in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is hidden in the documentation and if you don't pay full attention it is really hard to realize that. I guess that is a result on how the bashrc and bash_profile are designed to work, but I don't think everybody uses them correctly. Furthermore, Mac is the exception and they don't follow those rules... fun fun!
@@ -0,0 +1,53 @@ | |||
#!/usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this private launcher script used for? Registering plugins on test environments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking back over the earlier elements of the commit and maybe making a bit more sense to me now. But what exactly is a private task in the context of the typical qiita execution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The private launcher is a notion that I've introduced for first time in this PR. The scripts qiita-private-launcher
and qiita-plugin-launcher
are two scripts tailored to be optimized by the system administrators so they decide which is the best way to execute parallel jobs. In the code added to the repository, these scripts are optimized to work in a laptop environment. In our servers, however, these scripts have been modified so instead of doing a direct bash execution, they execute a qsub submitting the job to the queue.
The private launcher is a script that used to submit jobs that are performing internal Qiita processes. The only functionality in which they're used at this moment is to complete the jobs. When a job is completed, artifacts are created (which involves file movement) so it is important to perform this in a different job rather than in the webserver itself. The main reason to use a different script instead of using qiita-plugins-launcher is that the internal Qiita jobs are more controlled and in general way lighter, so having a different script allows optimization of submission parameters of this jobs.
Hope this makes a bit more sense! And thanks for asking, this forces me to try to put in words the concepts that I have on my head.
Seems like all comments have been addressed. Merging. |
The PRs in other packages was failing because moi was not submitting the job. This has been a recurrent problem lately (during the Qiita workshop, a couple of deploys later, plugins tests...) so we (@antgonza and I) decided to not use moi when completing the job.
Instead, the job will be submitted to the queue (or whatever the Qiita admin actually wants) using a configuration script in a similar way that plugins are started.