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

Parsl QueueAdapter #80

Merged
merged 7 commits into from Oct 30, 2018
Merged

Parsl QueueAdapter #80

merged 7 commits into from Oct 30, 2018

Conversation

dgasmith
Copy link
Contributor

Description

Adds an initial Parsl QueueAdapter integration. Parsl is a pretty neat new workflow engine that is potentially extremely powerful for us as it has integrations to a large number of interfaces.

Status

  • Ready to go

@dgasmith dgasmith added Enhancement Project enhancement discussion Queue Related to the Queue system labels Oct 26, 2018
@codecov
Copy link

codecov bot commented Oct 26, 2018

Codecov Report

Merging #80 into master will increase coverage by 16.98%.
The diff coverage is 91.66%.

@QCArchiveBot
Copy link
Collaborator

This pull request introduces 1 alert when merging a8b542a into 03177f2 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

Comment posted by LGTM.com

@dgasmith dgasmith added this to the v0.3.0b milestone Oct 28, 2018
Copy link
Collaborator

@Lnaden Lnaden left a comment

Choose a reason for hiding this comment

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

One request: Can you update the README.md file in the devtools/conda-envs folder to reflect the new env you provide?

Otherwise LGTM, the rebase with PyDantic might take some additional work, converting dict keys to class attrs, but should be straightforward

@@ -973,7 +973,7 @@ def get_managers(self, query, projection=None):

### Users

def add_user(self, username, password, permissions=["read"]):
def add_user(self, username, password, permissions=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you did not want to set this to None just to make the kwarg default mutable, you could set a tuple as the default which would fix the issue. Not required though for this pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Project enhancement discussion Queue Related to the Queue system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants