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