Skip to content

Commit

Permalink
fix tests, tweak logic
Browse files Browse the repository at this point in the history
  • Loading branch information
emmyoop committed May 23, 2024
1 parent 8a1b6a4 commit 7406809
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
9 changes: 7 additions & 2 deletions dbt_common/record.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ def load(cls, file_name: str) -> Dict[str, List[Record]]:
records_by_type: Dict[str, List[Record]] = {}

for record_type_name in loaded_dct:
# TODO: this break with QueryRecord on replay since it's
# not in common so isn't part of cls._record_cls_by_name yet

record_cls = cls._record_cls_by_name[record_type_name]
rec_list = []
for record_dct in loaded_dct[record_type_name]:
Expand Down Expand Up @@ -164,7 +167,8 @@ def get_record_mode_from_env() -> Optional[RecorderMode]:

if record_mode.lower() == "record":
return RecorderMode.RECORD
elif record_mode.lower() == "replay":
# replaying requires a file path, otherwise treat as noop
elif record_mode.lower() == "replay" and os.environ["DBT_RECORDER_REPLAY_PATH"] is not None:
return RecorderMode.REPLAY

# if you don't specify record/replay it's a noop
Expand All @@ -191,7 +195,8 @@ def get_record_types_from_env() -> Optional[List]:
# Types not defined in common are not in the record_types list yet
# TODO: is there a better way to do this without hardcoding? We can't just
# wait for later because if it's QueryRecord (not defined in common) we don't
# want to remove it to ensure everything else is filtered out....
# want to remove it to ensure everything else is filtered out.... This is also
# a problem with replaying QueryRecords generally
if type not in Recorder._record_cls_by_name and type != "QueryRecord":
print(f"Invalid record type: {type}") # TODO: remove after testing
record_types.remove(type)
Expand Down
6 changes: 5 additions & 1 deletion docs/guides/record_replay.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,16 @@ If `DBT_RECORDER_MODE` is not `replay` or `record`, case insensitive, this is a

`DBT_RECODER_TYPES` is optional. It indicates which types to filter the results by and expects a list of strings values for the `Record` subclasses. Any invalid types will be ignored. `all` is a valid type and behaves the same as not populating the env var.

example

```bash
DBT_RECORDER_MODE=record DBT_RECODER_TYPES=QueryRecord,GetEnvRecord dbt run
```

replay need the file to replay
```bash
DBT_RECORDER_MODE=replay DBT_RECORDER_REPLAY_PATH=recording.json dbt run
```

## Final Thoughts

We are aware of the potential limitations of this mechanism, since it makes several strong assumptions, not least of which are:
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_decorator_records():
prev = os.environ.get("DBT_RECORDER_MODE", None)
try:
os.environ["DBT_RECORDER_MODE"] = "Record"
recorder = Recorder(RecorderMode.RECORD)
recorder = Recorder(RecorderMode.RECORD, None)
set_invocation_context({})
get_invocation_context().recorder = recorder

Expand Down Expand Up @@ -56,7 +56,7 @@ def test_decorator_replays():
prev = os.environ.get("DBT_RECORDER_MODE", None)
try:
os.environ["DBT_RECORDER_MODE"] = "Replay"
recorder = Recorder(RecorderMode.REPLAY)
recorder = Recorder(RecorderMode.REPLAY, None)
set_invocation_context({})
get_invocation_context().recorder = recorder

Expand Down

0 comments on commit 7406809

Please sign in to comment.