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

FEATURE: Add testcase #385

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

FEATURE: Add testcase #385

wants to merge 2 commits into from

Conversation

dam1r89
Copy link

@dam1r89 dam1r89 commented Feb 3, 2023

Context

With this test case, I'm reproducing the problem that I'm facing. If the request matcher is ignoring some parts of the request making them the same, those requests should increment the same key in the $indexTable.

What has been done

  • Failing test explaining the problem that I have

How to test

  • Run the test

Notes

If you are open for accepting it, I could submit a fix for this issue.

@higidi higidi added Bug Something isn't working Status: Needs review labels Feb 20, 2023
@higidi
Copy link
Contributor

higidi commented Feb 20, 2023

Hi @dam1r89

thx 4 contributing! I think i understood the problem.

What do you think about changing or implementing a new request matching api to return something like a hash (same as Request::getHash() does before) to still allow using of an index-table instead persisting and matching all requests again and again when a new arrives?

I'm asking because on long running processes this might get a problem.

What do you think?

@higidi higidi self-assigned this Feb 20, 2023
@dam1r89
Copy link
Author

dam1r89 commented Feb 28, 2023

At the moment, I am occupied with other tasks and cannot say when I will have the time to review this.

I am curious about the scale of the tests you referenced - are they limited to hundreds of requests?

It may not be necessary to pursue optimization unless it becomes a significant issue, as the majority of tests will likely not generate a large volume of requests.

@dam1r89
Copy link
Author

dam1r89 commented Oct 28, 2023

Hey @higidi, any update on this? Just to let you know, we are currently using the version with this patch without any problems on a fairly large code base.

I wanted to check in to see if you would be open to accepting another PR with an additional feature that I need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Status: Needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants