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

Include exposures in RPC compile results #3702

Closed
owlas opened this issue Aug 6, 2021 · 3 comments
Closed

Include exposures in RPC compile results #3702

owlas opened this issue Aug 6, 2021 · 3 comments
Labels
enhancement New feature or request rpc Issues related to dbt's RPC server

Comments

@owlas
Copy link

owlas commented Aug 6, 2021

Describe the feature

When you invoke the compile method on a dbt rpc server, it returns exposures:

1. I compile a job

### Compile job
POST http://0.0.0.0:8580/jsonrpc
Content-Type: application/json

{
  "jsonrpc": "2.0",
  "id": "{{$uuid}}",
  "method": "compile",
  "params": {}
}

2. I get the results

### Poll for job results
POST http://localhost:8580/jsonrpc
Content-Type: application/json

{
  "jsonrpc": "2.0",
  "id": "{{$uuid}}",
  "method": "poll",
  "params": {
    "request_token": "{{request_token}}"
  }
}

3. I get a response that includes a node with an exposure

  "result": {
    "state": "success",
    "start": "2021-08-06T09:42:50.241257Z",
    "end": "2021-08-06T09:42:53.341310Z",
    "elapsed": 3.100053,
    "logs": [...],
    "tags": null,
    "results": [
      {
        "status": "success",
        "timing": [...],
        "thread_id": "Thread-1",
        "execution_time": 0.0025038719177246094,
        "adapter_response": {},
        "message": null,
        "failures": null,
        "node":          <<<< CAN BE AN EXPOSURE

Describe alternatives you've considered

Alternatively: don't include them.

Additional context

The contracts that are relevant are here. I'm probably missing something but why is the ParsedExposure a GraphMemberNode and not a CompileResultNode ? Only the latter is included in the dbt rpc contract.

https://github.com/dbt-labs/dbt/blob/749f87397ec1e0a270b2e09bd8dbeb71862fdb81/core/dbt/contracts/graph/compiled.py#L184-L225

Who will this benefit?

dbt rpc users that are using exposures

Are you interested in contributing this feature?

Happy to help

@owlas owlas added enhancement New feature or request triage labels Aug 6, 2021
@owlas owlas changed the title RPC compile contract doesn't include exposure Include exposures in RPC compile results Aug 6, 2021
@jtcohen6
Copy link
Contributor

Thanks for opening @owlas! I really appreciated reading the context over in lightdash/lightdash#202, and I'm excited to see your interest in leveraging exposures for a very cool application. Let's think about how dbt can best get you the info you're after.

Core change

Sources and exposures are a bit funny today, and unlike all other other nodes in dbt. dbt doesn't include either one in the results from compile, because neither of these node types is (technically) executable: they don't have any SQL to compile, or any database object to be creating.

https://github.com/dbt-labs/dbt/blob/1a984601ee964fff59e338805ab49a1e8489e342/core/dbt/node_types.py#L20-L30

https://github.com/dbt-labs/dbt/blob/1a984601ee964fff59e338805ab49a1e8489e342/core/dbt/task/compile.py#L52-L57

One option here: compile everything. Redefine the compile task to include all node types. Remove the distinction between CompileResultNode and GraphMemberNode. Include both sources + exposures in the category of, "If this node passed to a compile() call, just return its parsed representation unchanged." (That's how sources work today.)

Incidentally, if we made that change, and also the one in #3716, I don't believe we'll need the executable designation any more.

Other RPC methods

How much control do you have over the RPC methods that you're submitting? If what you're after is the full (or partial) contents of the manifest, perhaps you could use one of these other methods:

  • list (around since v0.20): This is a very quick, synchronous method that returns info directly from the manifest, for any/all nodes fitting the supplied selection criteria, including exposures. It only includes a subset of node properties. When we added this in a few months ago (Add keys to list output, parametrize #3356), we considered, but didn't go so far as to implement, an additional parameter that could customize which keys were returned. I'd still be supportive of that.
  • get-manifest (around since v0.17): This is a weird and undocumented method, originally implemented as one potential approach to supporting interactive dbt-docs. It may just be the thing you need. This method compiles all compilable nodes in the project, then returns the entire manifest contents in a manifest key. That includes manifest.exposures.

@jtcohen6 jtcohen6 added rpc Issues related to dbt's RPC server and removed triage labels Aug 12, 2021
@owlas
Copy link
Author

owlas commented Sep 2, 2021

Hey @jtcohen6

Thanks for the incredibly detailed answer.

Sources and exposures are a bit funny today

I didn't fully appreciate this but I can see now why they aren't there. I'm not sure what the implications would be for bringing them into the compile process and dropping executable. However...

get-manifest (around since v0.17): This is a weird and undocumented method

This is exactly what we need, this also brings a bunch of additional metadata that we've been craving. Is this stable enough to rely on at the moment? It seems it's been around for a while as you say.

Just testing get-manifest now in place of compile for Lightdash: lightdash/lightdash#409

list

This is also really useful and we'll use this where we can to avoid a full re-compile.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Sep 3, 2021

get-manifest (around since v0.17): This is a weird and undocumented method

This is exactly what we need, this also brings a bunch of additional metadata that we've been craving. Is this stable enough to rely on at the moment? It seems it's been around for a while as you say.

I don't foresee us ripping out get-manifest anytime soon, and the method itself is quite straightforward. I would just reiterate what I said when you and I had a chance to talk, which is that the RPC server overall does not have the same commitments/guarantees as dbt-core, we're soon to split it out of this codebase (#3501, #3675), and we have plans to significantly rework it over the medium term.

In the meantime, I'm going to close this issue :)

@jtcohen6 jtcohen6 closed this as completed Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rpc Issues related to dbt's RPC server
Projects
None yet
Development

No branches or pull requests

2 participants