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.

Sign up for GitHub

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

Jump to bottom

Issues/4455 support mesos roles #4456

Closed
wants to merge 4 commits into from
Closed

Issues/4455 support mesos roles #4456

wants to merge 4 commits into from

Conversation

stephanaime
Copy link
Contributor

Changelog Entry

To be copied to the draft changelog by merger:

  • PR submitter writes their recommendation for a changelog entry
    Allow use of mesos roles

Reviewer Checklist

  • Make sure it is coming from issues/XXXX-fix-the-thing in the Toil repo, or from an external repo.
    • If it is coming from an external repo, make sure to pull it in for CI with: 
      contrib/admin/test-pr otheruser theirbranchname issues/XXXX-fix-the-thing
    • If there is no associated issue, create one.
  • Read through the code changes. Make sure that it doesn't have:
    • Addition of trailing whitespace.
    • New variable or member names in camelCase that want to be in snake_case.
    • New functions without type hints.
    • New functions or classes without informative docstrings.
    • Changes to semantics not reflected in the relevant docstrings.
    • New or changed command line options for Toil workflows that are not reflected in docs/running/{cliOptions,cwl,wdl}.rst
    • New features without tests.
  • Comment on the lines of code where problems exist with a review comment. You can shift-click the line numbers in the diff to select multiple lines.
  • Finish the review with an overall description of your opinion.

Merger Checklist

  • Make sure the PR passes tests.
  • Make sure the PR has been reviewed since its last modification. If not, review it.
  • Merge with the Github "Squash and merge" feature.
    • If there are multiple authors' commits, add Co-authored-by to give credit to all contributing authors.
  • Copy its recommended changelog entry to the Draft Changelog.
  • Append the issue number in parentheses to the changelog entry.

DailyDreaming
DailyDreaming requested changes May 2, 2023
View reviewed changes
version_template.py
@@ -28,7 +28,7 @@
# - don't import even standard modules at global scope without renaming them
# to have leading/trailing underscores

baseVersion = '5.10.0a1'
baseVersion = '5.9.2'
Copy link
Member

Choose a reason for hiding this comment

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

Changed by accident?

adamnovak
adamnovak requested changes May 4, 2023
View reviewed changes
src/toil/batchSystems/mesos/batchSystem.py
@@ -94,6 +94,9 @@ def __init__(self, config, maxCores, maxMemory, maxDisk):

# Address of the Mesos master in the form host:port where host can be an IP or a hostname
self.mesos_endpoint = config.mesos_endpoint
self.mesos_role = config.mesos_role
self.mesos_name = config.mesos_name
self.mesos_framework_id = config.mesos_framework_id
Copy link
Member

Choose a reason for hiding this comment

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

I think this is never read.

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait maybe Scheduler uses it.

@adamnovak adamnovak linked an issue May 4, 2023 that may be closed by this pull request
Allow the use of meso roles #4455
Closed
@adamnovak adamnovak mentioned this pull request May 4, 2023
Merged
@adamnovak
Copy link
Member

I merged this in #4464.

@adamnovak adamnovak closed this May 5, 2023
@Fifan31
Copy link

Fifan31 commented May 9, 2023

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamnovak adamnovak adamnovak requested changes

@DailyDreaming DailyDreaming DailyDreaming requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow the use of meso roles
4 participants
@stephanaime @adamnovak @Fifan31 @DailyDreaming