Skip to content

Conversation

@fjetter
Copy link
Member

@fjetter fjetter commented Apr 16, 2025

See #9043

@fjetter fjetter changed the title Cache WorkerState host Cache WorkerState host property Apr 16, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

distributed/scheduler.py:569

  • Caching the host property at initialization may lead to inconsistencies if self.address changes later. Please confirm that the worker's address remains constant after initialization.
self.host = get_address_host(self.address)

distributed/scheduler.py:587

  • Removing the dynamic host property in favor of a cached value requires ensuring that no dependent code relies on its dynamic computation. Verify that this change will not affect behaviors expecting an updated host value.
@property
    def host(self) -> str:

@github-actions
Copy link
Contributor

github-actions bot commented Apr 16, 2025

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    27 files  ±0      27 suites  ±0   11h 21m 52s ⏱️ + 5m 42s
 4 100 tests ±0   3 981 ✅  - 1    112 💤 ±0  7 ❌ +1 
51 400 runs  ±0  49 085 ✅ ±0  2 308 💤  - 1  7 ❌ +1 

For more details on these failures, see this check.

Results for commit 53fad01. ± Comparison against base commit 16aa189.

♻️ This comment has been updated with latest results.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- test failures seem unrelated

@fjetter fjetter merged commit 08f01fa into dask:main Apr 16, 2025
26 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants