have AzureComputeLogManager implement CapturedLogManager interface
Needs ReviewPublic
Actions

Authored by • prha on Jul 12 2021, 9:59 PM.

Details

Reviewers

alangenfeld
johann
• max

Summary

depends on D8385, D8840

Test Plan

Diff Detail

Repository

R1 dagster

Lint

Lint Passed

Unit

No Test Coverage

Event Timeline

• prha created this revision.Jul 12 2021, 9:59 PM

Harbormaster completed remote builds in B33598: Diff 41475.Jul 12 2021, 10:24 PM

• prha requested review of this revision.Jul 12 2021, 10:24 PM

alangenfeld added inline comments.Jul 13 2021, 8:04 PM

python_modules/libraries/dagster-azure/dagster_azure/blob/compute_log_manager.py
73–75	definitely odd pattern
python_modules/libraries/dagster-azure/dagster_azure_tests/blob_tests/test_compute_log_manager.py
147–149	can you not just mock out the free function in the module or does it break?

• prha added inline comments.Jul 13 2021, 9:09 PM

python_modules/libraries/dagster-azure/dagster_azure_tests/blob_tests/test_compute_log_manager.py
147–149	Unfortunately, the scope of the mocking function ends after the log manager is created, not when it is applied in the `capture_logs` contextmanager.

rebase

Harbormaster completed remote builds in B33750: Diff 41673.Jul 15 2021, 5:16 PM

• max resigned from this revision.Jul 21 2021, 11:44 PM

Revision Contents
Changeset List

Path

Size

python_modules/

libraries/

dagster-azure/

dagster_azure/

blob/

compute_log_manager.py

80 lines

dagster_azure_tests/

blob_tests/

test_compute_log_manager.py

33 lines

Diff 41673

View Options

python_modules/libraries/dagster-azure/dagster_azure/blob/compute_log_manager.py