Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(batching): Exclude operations that failed pre-executions #1942

Merged
merged 3 commits into from
Apr 1, 2024

Conversation

samuelAndalon
Copy link
Contributor

@samuelAndalon samuelAndalon commented Apr 1, 2024

📝 Description

Remove operations that failed pre-executions from batching state, an operation can become invalid if parsing, validation or any other logic from preparsed document provider failed (such as persisted queries).

@dariuszkuc
Copy link
Collaborator

Changes look fine but wondering whether whole batch should be rejected instead.

@samuelAndalon
Copy link
Contributor Author

samuelAndalon commented Apr 1, 2024

Changes look fine but wondering whether whole batch should be rejected instead.

thought about that, but IMO we should execute as much as we can, if an operation in the batch failed pre-execution, we should fulfill the contract of operations being independent.

@dariuszkuc dariuszkuc merged commit ddc81be into master Apr 1, 2024
8 checks passed
@dariuszkuc dariuszkuc deleted the feat/batching-apq branch April 1, 2024 20:11
samuelAndalon added a commit that referenced this pull request Apr 2, 2024
Remove operations that failed pre-executions from batching state, an
operation can become invalid if parsing, validation or any other logic
from preparsed document provider failed (such as persisted queries).
samuelAndalon added a commit that referenced this pull request Apr 2, 2024
…#1946)

### 📝 Description

Remove operations that failed pre-executions from batching state, an
operation can become invalid if parsing, validation or any other logic
from preparsed document provider failed (such as persisted queries).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants