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

Workaround for Kerberos bug in slurm sbatch --parsable (solves #148) #149

Merged
merged 3 commits into from
Jul 24, 2019

Conversation

Hoeze
Copy link
Contributor

@Hoeze Hoeze commented Jul 7, 2019

In addition, added a missing configuration option to README.md which took me two hours to debug

@rkdarst
Copy link
Contributor

rkdarst commented Jul 8, 2019

I'd say this looks good. Really this splitlines thing could be applied many more places, since if it's one line it doesn't matter, if there is random messages they will usually be at the start. But that's something for another time.

For the API handler thing, I'd recommend leaving it as in #126. It's conceivable that the handler names and what needs to be written could change in the future (especially since this is still new and developing), and just importing it will always work in the future. In fact I'd recommend just importing batchspawner, not batchspawner.api, for a similar reason...

@Hoeze
Copy link
Contributor Author

Hoeze commented Jul 9, 2019

Thanks for the review @rkdarst. I changed the readme accordingly.

@rkdarst
Copy link
Contributor

rkdarst commented Jul 16, 2019

Thanks, I merged this into my "dev" branch so it will get taken when #144 goes in. Looks like the import batchspawner was also in #130, so I removed one duplicate.

@mbmilligan mbmilligan merged commit 70f4992 into jupyterhub:master Jul 24, 2019
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