-
Notifications
You must be signed in to change notification settings - Fork 242
New AWS jobstore. #5123
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
Open
DailyDreaming
wants to merge
17
commits into
master
Choose a base branch
from
issues/964-aws-remove-sdb
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
New AWS jobstore. #5123
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
b0b1752
New aws jobstore.
DailyDreaming 7dc1a7c
Update.
DailyDreaming 06b7a40
Updates.
DailyDreaming 4c12449
Linting.
DailyDreaming 2900c99
Update.
DailyDreaming 3a345dd
Update from master.
DailyDreaming 9322285
Update.
DailyDreaming 031bab4
Update and rebase.
DailyDreaming c58e025
Update and rebase.
DailyDreaming 698b450
Assuage make docs's anger.
DailyDreaming b9f3cc8
Rebase.
DailyDreaming 321a2c6
Some compat, some review comments.
DailyDreaming faf0581
Move boto imports.
DailyDreaming a9a8880
Update.
DailyDreaming 082311d
Update comments, move imports, and update docstrings.
DailyDreaming 0608622
Update imports.
DailyDreaming 687b9b7
Merge branch 'master' into issues/964-aws-remove-sdb
DailyDreaming File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ | |
Optional, | ||
Union, | ||
cast, | ||
overload, | ||
overload | ||
) | ||
from urllib.error import HTTPError | ||
from urllib.parse import ParseResult, urlparse | ||
|
@@ -88,7 +88,7 @@ def __init__(self, url: ParseResult) -> None: | |
class NoSuchJobException(Exception): | ||
"""Indicates that the specified job does not exist.""" | ||
|
||
def __init__(self, jobStoreID: FileID): | ||
def __init__(self, jobStoreID: Union[FileID, str]): | ||
""" | ||
:param str jobStoreID: the jobStoreID that was mistakenly assumed to exist | ||
""" | ||
|
@@ -98,7 +98,7 @@ def __init__(self, jobStoreID: FileID): | |
class ConcurrentFileModificationException(Exception): | ||
"""Indicates that the file was attempted to be modified by multiple processes at once.""" | ||
|
||
def __init__(self, jobStoreFileID: FileID): | ||
def __init__(self, jobStoreFileID: Union[FileID, str]): | ||
""" | ||
:param jobStoreFileID: the ID of the file that was modified by multiple workers | ||
or processes concurrently | ||
|
@@ -110,7 +110,7 @@ class NoSuchFileException(Exception): | |
"""Indicates that the specified file does not exist.""" | ||
|
||
def __init__( | ||
self, jobStoreFileID: FileID, customName: Optional[str] = None, *extra: Any | ||
self, jobStoreFileID: Union[FileID, str], customName: Optional[str] = None, *extra: Any | ||
): | ||
""" | ||
:param jobStoreFileID: the ID of the file that was mistakenly assumed to exist | ||
|
@@ -138,11 +138,12 @@ class NoSuchJobStoreException(LocatorException): | |
def __init__(self, locator: str, prefix: str): | ||
""" | ||
:param str locator: The location of the job store | ||
:param str prefix: The type of job store | ||
""" | ||
super().__init__( | ||
"The job store '%s' does not exist, so there is nothing to restart.", | ||
locator, | ||
prefix, | ||
prefix | ||
) | ||
|
||
|
||
|
@@ -157,7 +158,7 @@ def __init__(self, locator: str, prefix: str): | |
"The job store '%s' already exists. Use --restart to resume the workflow, or remove " | ||
"the job store with 'toil clean' to start the workflow from scratch.", | ||
locator, | ||
prefix, | ||
prefix | ||
) | ||
|
||
|
||
|
@@ -224,7 +225,7 @@ def write_config(self) -> None: | |
) as fileHandle: | ||
pickle.dump(self.__config, fileHandle, pickle.HIGHEST_PROTOCOL) | ||
|
||
def resume(self) -> None: | ||
def resume(self, sse_key_path: Optional[str] = None) -> None: | ||
""" | ||
Connect this instance to the physical storage it represents and load the Toil configuration | ||
into the :attr:`AbstractJobStore.config` attribute. | ||
|
@@ -748,7 +749,6 @@ def _open_url(cls, url: ParseResult) -> IO[bytes]: | |
""" | ||
raise NotImplementedError(f"No implementation for {url}") | ||
|
||
@classmethod | ||
@abstractmethod | ||
def _write_to_url( | ||
cls, | ||
|
@@ -1155,15 +1155,6 @@ def assign_job_id(self, job_description: JobDescription) -> None: | |
""" | ||
raise NotImplementedError() | ||
|
||
@contextmanager | ||
def batch(self) -> Iterator[None]: | ||
""" | ||
If supported by the batch system, calls to create() with this context | ||
manager active will be performed in a batch after the context manager | ||
is released. | ||
""" | ||
yield | ||
|
||
@deprecated(new_function_name="create_job") | ||
def create(self, jobDescription: JobDescription) -> JobDescription: | ||
return self.create_job(jobDescription) | ||
|
@@ -1261,6 +1252,15 @@ def load_job(self, job_id: str) -> JobDescription: | |
def update(self, jobDescription: JobDescription) -> None: | ||
return self.update_job(jobDescription) | ||
|
||
@contextmanager | ||
def batch(self) -> Iterator[None]: | ||
""" | ||
If supported by the batch system, calls to create() with this context | ||
manager active will be performed in a batch after the context manager | ||
is released. | ||
""" | ||
yield | ||
|
||
@abstractmethod | ||
def update_job(self, job_description: JobDescription) -> None: | ||
""" | ||
|
@@ -1502,6 +1502,7 @@ def read_file_stream( | |
) -> ContextManager[IO[str]]: ... | ||
|
||
@abstractmethod | ||
@contextmanager # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we really only need/want |
||
def read_file_stream( | ||
self, | ||
file_id: Union[FileID, str], | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 there a good reason to say this can be a string now? Is it too hard to drag the typed file ID object through in the new implementation for some good reason?
When this is a string, is this meant to be the string-packed version of the file ID? Or just the ID part without e.g. the size packed in?
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.
I guess some of the user-facing file ID API still accepts file IDs typed as strings, so maybe we do still need to be able to take them here.