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

Grab environment variables needed for grid engine #137

Merged
merged 1 commit into from Jul 23, 2020

Conversation

jekriske-lilly
Copy link
Contributor

When running as a service, the path for qsub/qstat/qdel commands is needed in the environment. In addition, several SGE environment variables are needed in order to submit jobs to the cluster. These are mentioned in the SGE install guides and are carried over to Univa Grid Engine as well.

Here's an older admin guide (page 7) describing the variables

Tested with UGE-8.6.4

@rkdarst
Copy link
Contributor

rkdarst commented Jun 14, 2019

Does anyone know how the GridengineSpawner could work without this? Does everyone else just manually specify additional keepenv, or could there be some other local differences here? While I don't see anything wrong here, I'm curious about the big picture to ensure we don't break things for other people.

Are you interested in being a grid engine maintainer?

@jekriske-lilly
Copy link
Contributor Author

I'm not sure how others are doing this unless they are doing something custom to set those env variables within the service and use keepvars. Without these, they should be greeted with the super helpful error from qsub:

Unable to initialize environment because of error: error without diagnosis message
Exiting.

I don't mind helping to maintain the grid engine piece. We have 1000+ users on our cluster, keeping this working properly is super important.

@cbjhnsn you mentioned your group is using SGE. Would you foresee any issues in your environment with the approach in this PR?

This was referenced Jul 23, 2020
@mbmilligan
Copy link
Member

This seems well contained to the GridEngineSpawner. Not seeing any dissenting opinions, accepting.

@mbmilligan mbmilligan merged commit 06ce979 into jupyterhub:master Jul 23, 2020
@welcome
Copy link

welcome bot commented Jul 23, 2020

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

mbmilligan added a commit that referenced this pull request Jul 23, 2020
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.

None yet

3 participants