Skip to content

Update config handling to rely on async ExecutorFileSystem#15408

Closed
pakrym-oai wants to merge 1 commit intomainfrom
pakry/config-test
Closed

Update config handling to rely on async ExecutorFileSystem#15408
pakrym-oai wants to merge 1 commit intomainfrom
pakry/config-test

Conversation

@pakrym-oai
Copy link
Collaborator

Summary

  • Align config-related changes with the requested async ExecutorFileSystem loading flow
  • Ensure no sync ::fs:: reads remain in core/config outside tests

Testing

  • Not run (not requested)

fn load_catalog_json(path: &AbsolutePathBuf) -> std::io::Result<ModelsResponse> {
let file_contents = std::fs::read_to_string(path)?;
fn absolute_path_for_filesystem_read(path: &Path) -> std::io::Result<AbsolutePathBuf> {
let absolute_path = if path.is_absolute() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe is_absolute() is platform-dependent.

})
}

async fn read_text_file(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is there a helper for this? Shouldn't ExecutorFileSystem provide this directly?

return Ok(());
}

write_atomically(&write_paths.write_path, &document.doc.to_string()).with_context(|| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we lose atomic updates here, correct?

@pakrym-oai pakrym-oai closed this Mar 24, 2026
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