-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Feature: async rpc (#1706) #1735
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
d23b1aa
Clean up return types and return values for async rpc work + mypy
4cb4318
Implement async RPC (no gc of tasks)
5e52c64
gc, tests
d51ebac
remove windows support from RPC
3498065
Handle started/ended correctly
58fcbcf
Improve logging/error handling
5937c20
bigquery error handling: log strings, not Exception objects
0563061
More flexible json.dumps in rpc
9ea6e8d
flake8
2cf2c82
add early error on invalid parameters via inspect shenanigans
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 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 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 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 |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
from dbt.contracts.graph.unparsed import Time, FreshnessStatus | ||
from dbt.contracts.graph.parsed import ParsedSourceDefinition | ||
from dbt.contracts.util import Writable | ||
from dbt.logger import LogMessage | ||
from hologram.helpers import StrEnum | ||
from hologram import JsonSchemaMixin | ||
|
||
|
@@ -77,6 +78,15 @@ class ExecutionResult(JsonSchemaMixin, Writable): | |
generated_at: datetime | ||
elapsed_time: Real | ||
|
||
def __len__(self): | ||
return len(self.results) | ||
|
||
def __iter__(self): | ||
return iter(self.results) | ||
|
||
def __getitem__(self, idx): | ||
return self.results[idx] | ||
|
||
|
||
# due to issues with typing.Union collapsing subclasses, this can't subclass | ||
# PartialResult | ||
|
@@ -137,6 +147,15 @@ def write(self, path, omit_none=True): | |
output = FreshnessRunOutput(meta=meta, sources=sources) | ||
output.write(path, omit_none=omit_none) | ||
|
||
def __len__(self): | ||
return len(self.results) | ||
|
||
def __iter__(self): | ||
return iter(self.results) | ||
|
||
def __getitem__(self, idx): | ||
return self.results[idx] | ||
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. These changes make results a lot like the old thing where we returned |
||
|
||
|
||
def _copykeys(src, keys, **updates): | ||
return {k: getattr(src, k) for k in keys} | ||
|
@@ -183,12 +202,18 @@ class RemoteCompileResult(JsonSchemaMixin): | |
compiled_sql: str | ||
node: CompileResultNode | ||
timing: List[TimingInfo] | ||
logs: List[LogMessage] | ||
|
||
@property | ||
def error(self): | ||
return None | ||
|
||
|
||
@dataclass | ||
class RemoteExecutionResult(ExecutionResult): | ||
logs: List[LogMessage] | ||
|
||
|
||
@dataclass | ||
class ResultTable(JsonSchemaMixin): | ||
column_names: List[str] | ||
|
This file contains 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 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 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 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 |
---|---|---|
@@ -1,6 +1,43 @@ | ||
"""The `rpc` package handles most aspects of the actual execution of dbt's RPC | ||
server (except for the server itself and the client tasks, which are defined in | ||
the `task.remote` and `task.rpc_server` modules). | ||
|
||
The general idea from a thread/process management perspective (ignoring the | ||
--single-threaded flag!) is as follows: | ||
|
||
- The RPC server runs a web server, in particular `werkzeug`, which manages a | ||
thread pool. | ||
- When a request comes in, werkzeug spins off a thread to manage the | ||
request/response portion. dbt itself has basically no control over this | ||
operation - from our viewpoint request/response cycles are fully | ||
synchronous. | ||
- synchronous requests are defined as methods in the `TaskManager` and handled | ||
in the responding thread directly. | ||
- Asynchronous requests (defined in `tasks.remote`) are kicked off wrapped in | ||
`RequestTaskHandler`s, which manage a new process and a new thread. | ||
- The process runs the actual dbt request, logging via a message queue | ||
- eventually just before process exit, the process places an "error" or | ||
"result" on the queue | ||
- The thread monitors the queue, taking logs off the queue and adding them | ||
to the `RequestTaskHandler`'s `logs` attribute. | ||
- The thread also monitors the `is_alive` state of the process, in case | ||
it is killed "unexpectedly" (including via `kill`) | ||
- When the thread sees an error or result come over the queue, it join()s | ||
the process. | ||
- When the thread sees that the process has disappeared without placing | ||
anything on the queue, it checks the queue one last time, and then acts | ||
as if the queue received an 'Unexpected termination' error | ||
- `kill` commands pointed at an asynchronous task kill the process and allow | ||
the thread to handle cleanup and management | ||
- When the RPC server receives a shutdown instruction, it: | ||
- stops responding to requests | ||
- `kills` all processes (triggering the end of all processes, right!?) | ||
- exits (all remaining threads should die here!) | ||
""" | ||
from dbt.rpc.error import ( # noqa | ||
dbt_error, server_error, invalid_params, RPCException | ||
) | ||
from dbt.rpc.task import RemoteCallable, RemoteCallableResult # noqa | ||
from dbt.rpc.logger import RemoteCallableResult # noqa | ||
from dbt.rpc.task import RemoteCallable # noqa | ||
from dbt.rpc.task_manager import TaskManager # noqa | ||
from dbt.rpc.response_manager import ResponseManager # noqa |
This file contains 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
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.
I changed all
os.environ.get
->os.getenv
for grep-ability reasons when I was adding some secret magic threading-related flags to the RPC server.