You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/12/03 19:01:05 UTC
[GitHub] [apisix] XFarooqi opened a new pull request, #8453: Router
XFarooqi opened a new pull request, #8453:
URL: https://github.com/apache/apisix/pull/8453
### Description
Updated the default HTTP router from radixtree_uri to radixtree_host_uri to remove the confusion that which route will hit first. Let's say, we have two routes, the first one requires the host matches *.example.com, and the URI path is /anything, And the second one needs the host matches foo.example.com exactly, and the URI path is also /anything. In such a case, if we send an API request, we may hit the first route, which is counterintuitive.
This is also helpful as most of the users have been in confusion since they think Apache APISIX will consider the HTTP host match by default. Also updated the documentation reflecting the result of these changes. i.e, Comparatively low performance can be expected in radixtree_host_uri
Fixes #8354
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] XFarooqi commented on pull request #8453: chore: update the default HTTP router
Posted by GitBox <gi...@apache.org>.
XFarooqi commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1336473020
@tokers
Can you please help me with these failing tests, maybe some guidance?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #8453: chore: update the default HTTP router
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1407933445
The questions you asked are more relative to the test-nginx framework. Maybe you can get some help from their side and spend more time learning about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] XFarooqi commented on pull request #8453: chore: update the default HTTP router
Posted by GitBox <gi...@apache.org>.
XFarooqi commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1336830686
Can you please help me more about the match conditions, I have no good idea as I am very new to this repo.
But I am ready to complete the task.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #8453: chore: update the default HTTP router
Posted by GitBox <gi...@apache.org>.
tokers commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1337032606
>
Please read more about the [what is route](https://apisix.apache.org/docs/apisix/terminology/route/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] XFarooqi commented on pull request #8453: chore: update the default HTTP router
Posted by "XFarooqi (via GitHub)" <gi...@apache.org>.
XFarooqi commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1407594664
> > > Because you modified the default router, it will cause some test cases to fail.
> > > So you also need to change the test case at the same time, specify `radixtree_uri` for the failure case.
> >
> >
> > @membphis Could we give to Abdul useful links to any guidelines or tutorials on how to test things on Apache APISIX?
> > I think it is quite hard and risky to modify existing tests without any knowledge. If we do not have any learning materials, we should prepare for new joiners.
>
> Here it is: https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md
Thank you @spacewander for providing me with the link. I respect your time.
Because I have no prior experience if writing test cases, I am having the following issues in writing test cases:
1. APISIX uses test-nginx framework for testing and its documentation itself is complex to understand.
2. Reading the documentation, I understand how to write test cases, etc but I am not able to find out what changes should I have to make in failing test cases.
3. In the summary of failing test cases, there is some test that is shown as failed but when I see that file then it doesn't include those test cases. For example in `t/plugin/ai.`, Failed Tests are [3, 9, 12, 15, 19, 22, 25, 28, 41, 45-46] but when I check this file it has only 12 test cases.
4. In the log description of **Failed Test 4 of `'t/plugin/ai4.t`** there are so many things that I am totally unable to understand.
<img width="1440" alt="Screenshot 2023-01-29 at 1 06 54 PM" src="https://user-images.githubusercontent.com/58102434/215313612-03179f06-2afa-45cd-933e-6242b8afb985.png">
I genuinely want to contribute to APISIX and solve this issue and I will highly appreciate it if you can guide me with step-by-step procedures for the issue. Because it looks like I am running in a loop where I am learning everything individually but not able to connect them with others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Boburmirzo commented on pull request #8453: chore: update the default HTTP router
Posted by "Boburmirzo (via GitHub)" <gi...@apache.org>.
Boburmirzo commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1401732462
> Because you modified the default router, it will cause some test cases to fail.
>
> So you also need to change the test case at the same time, specify `radixtree_uri` for the failure case.
@membphis Could we give to Abdul useful links to any guidelines or tutorials on how to test things on Apache APISIX?
I think it is quite hard and risky to modify existing tests without any knowledge. If we do not have any learning materials, we should prepare for new joiners.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] XFarooqi closed pull request #8453: chore: update the default HTTP router
Posted by "XFarooqi (via GitHub)" <gi...@apache.org>.
XFarooqi closed pull request #8453: chore: update the default HTTP router
URL: https://github.com/apache/apisix/pull/8453
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #8453: chore: update the default HTTP router
Posted by GitBox <gi...@apache.org>.
tokers commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1336590207
I think some of them failed due to the router was changed and must add more match conditions for routes in these cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #8453: chore: update the default HTTP router
Posted by GitBox <gi...@apache.org>.
membphis commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1337043465
Because you modified the default router, it will cause some test cases to fail.
So you also need to change the test case at the same time, specify `radixtree_uri` for the failure case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #8453: chore: update the default HTTP router
Posted by "spacewander (via GitHub)" <gi...@apache.org>.
spacewander commented on PR #8453:
URL: https://github.com/apache/apisix/pull/8453#issuecomment-1407270652
> > Because you modified the default router, it will cause some test cases to fail.
> > So you also need to change the test case at the same time, specify `radixtree_uri` for the failure case.
>
> @membphis Could we give to Abdul useful links to any guidelines or tutorials on how to test things on Apache APISIX?
>
> I think it is quite hard and risky to modify existing tests without any knowledge. If we do not have any learning materials, we should prepare for new joiners.
Here it is: https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org