Skip to content

Fix bugs found while testing file-based state tracking - #1347

Merged
dmjb merged 2 commits into
mainfrom
enable-state-files
Aug 12, 2025
Merged

Fix bugs found while testing file-based state tracking#1347
dmjb merged 2 commits into
mainfrom
enable-state-files

Conversation

@dmjb

@dmjb dmjb commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

This PR was intended to enable file-based state tracking. However, I found some bugs during testing which involves touching many files, so this PR addresses these bugs without enabling file-based state tracking.

  1. I discovered that I left out a call to change the state to "running" once the proxy-runner is ready. Simply adding the state manager type led to a circular dependency, which required a bunch of refactoring of code. This is responsible for most of the diff here.
  2. The code which created the base directory for the status files was using the wrong path - this was fixed.
  3. I originally had separate methods for creating the initial status, and updating. This led to problems with the detached process flow (which would try to create the status twice, leading to an error). This PR changes the code to use a single method which creates or update.

@dmjb dmjb changed the title Enable file-based state tracking [DRAFT] Enable file-based state tracking Aug 11, 2025
@dmjb
dmjb force-pushed the enable-state-files branch 2 times, most recently from dd16ce4 to bcf1e06 Compare August 12, 2025 09:30
@dmjb dmjb changed the title [DRAFT] Enable file-based state tracking Fix bugs found while testing file-based state tracking Aug 12, 2025
@dmjb
dmjb force-pushed the enable-state-files branch from 65107ab to 519df60 Compare August 12, 2025 10:45
@dmjb
dmjb marked this pull request as ready for review August 12, 2025 10:59
This PR was intended to enable file-based state tracking. However, I found some
bugs during testing which involves touching many files, so this PR addresses
these bugs without enabling file-based state tracking.

1. I discovered that I left out a call to change the state to "running" once the
proxy-runner is ready. Simply adding the state manager type led to a circular
dependency, which required a bunch of refactoring of code. This is responsible
for most of the diff here.

2. The code which created the base directory for the status files was using the
wrong path - this was fixed.

3. I originally had separate methods for creating the initial status, and
updating. This led to problems with the detached process flow (which would try
to create the status twice, leading to an error). This PR changes the code to
use a single method which creates or update.
@dmjb
dmjb force-pushed the enable-state-files branch from 427576c to 3e07dd9 Compare August 12, 2025 11:03
@dmjb
dmjb merged commit e6e29dc into main Aug 12, 2025
17 checks passed
@dmjb
dmjb deleted the enable-state-files branch August 12, 2025 12:50
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