add query recording options #135
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
==========================================
- Coverage 64.81% 64.78% -0.03%
==========================================
Files 50 50
Lines 3106 3115 +9
==========================================
+ Hits 2013 2018 +5
- Misses 1093 1097 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
emmyoop
force-pushed
the
er/MNTL-268-dynamic-recording
branch
from
7382ad0 to
8a1b6a4
Compare
| @@ -147,17 +152,46 @@ def print_diffs(self) -> None: | |||
|
|
|||
|
|
|||
| def get_record_mode_from_env() -> Optional[RecorderMode]: | |||
There was a problem hiding this comment.
Not a blocker for the current PR but a general observation, I think we are basically implementing something similar to pydantic settings functionalities here. We should consider consolidating all env readings(other than the ones that are also flags in click commands) to use it.
There was a problem hiding this comment.
dbt-labs/dbt-core#10100 addresses this. Going to leave this as is for now.
tests/unit/test_record.py
Outdated
| @@ -76,6 +136,10 @@ def test_func(a: int, b: str, c: Optional[str] = None) -> str: | |||
|
|
|||
| finally: | |||
| if prev is None: | |||
| os.environ.pop("DBT_RECORD", None) | |||
| os.environ.pop("DBT_RECORDER_MODE", None) | |||
There was a problem hiding this comment.
track previous and removing it might be easier to read if we write a fixture with yield in the middle?
@pytest.fixture
def setup():
prev = os.environ.get("DBT_RECORDER_MODE", None)
yield
os.environ["DBT_RECORDER_MODE"] = prev
There was a problem hiding this comment.
This does not resolve #140 right? It just add the option part?
|
@ChenyuLInx the scope of #140 changed while working on this ticket. I've updated it to reflect that we're just going to filter by record type. So this will close #140. |
resolves #140 / MNTL-268
Description
Add option to record by record type.
Checklist
changie newto create a changelog entry