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

[CT-1508] When a seed succeeds, LogSeedResult shows result.status when it should show result.message #6250

Closed
jtcohen6 opened this issue Nov 15, 2022 · 1 comment
Assignees
Labels
bug Something isn't working logging
Milestone

Comments

@jtcohen6
Copy link
Contributor

Background

As a general rule, when a node finishes running and returns its result, the result.message is either the database-specific "success message" (short) or "error message" (verbose).

When a seed succeeds:

    {
      "status": "success",
      "timing": [...],
      "thread_id": "Thread-1 (worker)",
      "execution_time": 0.08353090286254883,
      "adapter_response": {
        "_message": "INSERT 5",
        "code": "INSERT",
        "rows_affected": 5
      },
      "message": "INSERT 5",
      "failures": null,
      "unique_id": "seed.test.test_seed"
    }

When a seed fails:

    {
      "status": "error",
      "timing": [],
      "thread_id": "Thread-1 (worker)",
      "execution_time": 0.022258996963500977,
      "adapter_response": {},
      "message": "Compilation Error in seed test_seed (seeds/test_seed.csv)\n  Row 1 has 3 values, but Table only has 2 columns.\n  \n  > in macro materialization_seed_default (macros/materializations/seeds/seed.sql)\n  > called by seed test_seed (seeds/test_seed.csv)",
      "failures": null,
      "unique_id": "seed.test.test_seed"
    }

Our habit has been:

  • success: show the success message inline
  • error: just the word ERROR inline, and print the full error message at invocation end

What changed

We standardized these in #6174. Specifically, the status being passed into LogSeedResult is now always result.status. Previously, in the case of seed "success," this was result.message (diff).

Example

Running dbt seed in v1.3:

14:53:04  1 of 1 START seed file dbt_jcohen.test_seed .................................... [RUN]
14:53:04  1 of 1 OK loaded seed file dbt_jcohen.test_seed ................................ [INSERT 5 in 0.08s]

Running dbt seed using latest main changes (1.4.0-a1):

14:53:16  1 of 1 START seed file dbt_jcohen.test_seed .................................... [RUN]
14:53:16  1 of 1 OK loaded seed file dbt_jcohen.test_seed ................................ [success in 0.09s]

Resolution

We should keep our old habit:

  • success: show the short database-specific success message inline
  • error: show the status (ERROR) inline, and the database-specific success message further down (invocation summary)

@gshank's proposal:

So probably we should save both 'status' and 'message' in the structured log and differentiate in the message construction

I noticed this while running dbt seed. We should also validate that this change hasn't occurred for other resource types.

@jtcohen6 jtcohen6 added bug Something isn't working logging labels Nov 15, 2022
@jtcohen6 jtcohen6 added this to the v1.4 milestone Nov 15, 2022
@github-actions github-actions bot changed the title When a seed succeeds, LogSeedResult shows result.status when it should show result.message [CT-1508] When a seed succeeds, LogSeedResult shows result.status when it should show result.message Nov 15, 2022
@gshank gshank self-assigned this Nov 15, 2022
@gshank
Copy link
Contributor

gshank commented Dec 7, 2022

This was fixed in pull request #6293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging
Projects
None yet
Development

No branches or pull requests

2 participants