Skip to content

feat: backup file on streaming#323

Merged
kwasniew merged 6 commits into
mainfrom
write-backup-file
Sep 9, 2025
Merged

feat: backup file on streaming#323
kwasniew merged 6 commits into
mainfrom
write-backup-file

Conversation

@kwasniew
Copy link
Copy Markdown
Contributor

@kwasniew kwasniew commented Aug 20, 2025

About the changes

Save full ygg engine state string to a backup file

Important files

Discussion points

}

private synchronized void handleStreamingUpdate(String data) {
synchronized void handleStreamingUpdate(String data) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for easier testing

engine.takeState(data);
// TODO: write backup when engine exposes current state

String currentState = engine.getState();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the output from getState happens to match what we write as a backup file content

@gastonfournier gastonfournier moved this from New to In Progress in Issues and PRs Aug 21, 2025
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Aug 21, 2025

Pull Request Test Coverage Report for Build 17576309622

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 79.446%

Totals Coverage Status
Change from base Build 17575012426: 0.2%
Covered Lines: 2467
Relevant Lines: 3033

💛 - Coveralls

@kwasniew
Copy link
Copy Markdown
Contributor Author

Reviewers: can you also check if the backup file is read on the initial failing streaming connection?

Copy link
Copy Markdown
Member

@chriswk chriswk left a comment

Choose a reason for hiding this comment

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

Go ahead :)

@github-project-automation github-project-automation Bot moved this from In Progress to Approved PRs in Issues and PRs Sep 9, 2025
Comment thread .github/workflows/pull_requests.yml Outdated
flag-name: run-jvm-${{ join(matrix.*, '-') }}
parallel: true
base-path: src/main/java
fail-on-error: ${{ matrix.os != 'macos' }}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

coverallsapp/homebrew-coveralls#65 (comment) following this advice before coveralls starts working with macos 15.5

@kwasniew kwasniew merged commit b293e4d into main Sep 9, 2025
9 checks passed
@kwasniew kwasniew deleted the write-backup-file branch September 9, 2025 08:20
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Issues and PRs Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants