Skip to content

feat: Add support for multiple writers for one query - #218

Merged
AndreyMarkinPPC merged 2 commits into
google:mainfrom
Vasist10:main
Dec 25, 2025
Merged

feat: Add support for multiple writers for one query#218
AndreyMarkinPPC merged 2 commits into
google:mainfrom
Vasist10:main

Conversation

@Vasist10

@Vasist10 Vasist10 commented Dec 7, 2025

Copy link
Copy Markdown
Contributor

This PR adds support for using multiple writers for a single query.
Now users can pass:

  • writers — a list of writer names
  • writers_parameters — matching list of parameter dicts

The executor now loops through all writer clients and runs each one.
Single-writer configs still work the same for backward compatibility.

issue - #193

I also added tests to confirm:

  • multiple writers are created
  • both writers run during execution
  • old single-writer behavior still works
  • config loading supports the new fields

@AndreyMarkinPPC AndreyMarkinPPC added the enhancement New feature or request label Dec 9, 2025
@AndreyMarkinPPC

Copy link
Copy Markdown
Member

Hi @Vasist10, thanks for the contribution. Unfortunately, unit/test_config.py::test_save_returns_correct_data fails due to the proposed change.

Apart from that I have a couple of comments:

  • Please do not mark writer property as deprecated.
  • writers property can be unified under writer with str | list[str] type. I understand that logically writers are better but from the real usage of garf multiple writers are edge cases so I'd not prefer to expose multiple options for writer/writers at this point.
  • keep writer_parameters only - there's no need for writers_parameters. Each writer can be instantiated with same named parameters as well as **kwargs, if anything extra is passed to writer initialization it will be safely ignored.

@Vasist10

Copy link
Copy Markdown
Contributor Author

@AndreyMarkinPPC i have unified the configuration for writer...and removed writers_parameters as suggested

@AndreyMarkinPPC

Copy link
Copy Markdown
Member

@Vasist10 Thanks, looks great!

@AndreyMarkinPPC
AndreyMarkinPPC merged commit 84c1440 into google:main Dec 25, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants