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: support patch in route module in order to support publish/offline #1049

Closed
wants to merge 8 commits into from

Conversation

liuxiran
Copy link
Contributor

@liuxiran liuxiran commented Dec 14, 2020

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • New feature provided

  • Related issues

BE of #837
support for #991
borrow from #1005
depends on APISIX release with apache/apisix#2737 and sync https://github.com/apache/apisix-dashboard/blob/master/api/conf/schema.json

@liuxiran liuxiran mentioned this pull request Dec 14, 2020
1 task
@liuxiran liuxiran force-pushed the refactor_publish_offline_be branch from 2768523 to 8ce330b Compare December 14, 2020 14:12
@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #1049 (80692e0) into master (7c5fe95) will decrease coverage by 0.47%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1049      +/-   ##
==========================================
- Coverage   40.13%   39.66%   -0.48%     
==========================================
  Files          26       26              
  Lines        1572     1591      +19     
==========================================
  Hits          631      631              
- Misses        850      869      +19     
  Partials       91       91              
Impacted Files Coverage Δ
api/internal/core/entity/entity.go 0.00% <ø> (ø)
api/internal/handler/route/route.go 42.30% <0.00%> (-4.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c5fe95...80692e0. Read the comment docs.

func TestRoute_Patch(t *testing.T) {
tests := []HttpTestCase{
{
caseDesc: "route patch",
Copy link
Member

Choose a reason for hiding this comment

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

we should always create the related resource first.

eg: the route resource

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@membphis
Copy link
Member

It seems that the E2E test cases is unstable, @nic-chen please take a look:

--- FAIL: TestRoute_Create_Service (0.93s)
Error:     printer.go:54: GET http://127.0.0.1:9080/server_port
Error:     printer.go:54: PUT http://127.0.0.1:9000/apisix/admin/services/200
Error:     printer.go:54: PUT http://127.0.0.1:9000/apisix/admin/routes/r1
    route_service_upstream_test.go:190: 
        	Error Trace:	route_service_upstream_test.go:190
        	Error:      	Not equal: 
        	            	expected: 6
        	            	actual  : 7
        	Test:       	TestRoute_Create_Service
    route_service_upstream_test.go:192: 
        	Error Trace:	route_service_upstream_test.go:192
        	Error:      	Not equal: 
        	            	expected: 6
        	            	actual  : 5
        	Test:       	TestRoute_Create_Service

@nic-chen
Copy link
Member

It seems that the E2E test cases is unstable, @nic-chen please take a look:

--- FAIL: TestRoute_Create_Service (0.93s)
Error:     printer.go:54: GET http://127.0.0.1:9080/server_port
Error:     printer.go:54: PUT http://127.0.0.1:9000/apisix/admin/services/200
Error:     printer.go:54: PUT http://127.0.0.1:9000/apisix/admin/routes/r1
    route_service_upstream_test.go:190: 
        	Error Trace:	route_service_upstream_test.go:190
        	Error:      	Not equal: 
        	            	expected: 6
        	            	actual  : 7
        	Test:       	TestRoute_Create_Service
    route_service_upstream_test.go:192: 
        	Error Trace:	route_service_upstream_test.go:192
        	Error:      	Not equal: 
        	            	expected: 6
        	            	actual  : 5
        	Test:       	TestRoute_Create_Service

en, I have been working on it.

@LiteSun LiteSun requested review from nic-chen and tokers December 17, 2020 06:06
@nic-chen
Copy link
Member

CI failed @liuxiran

@liuxiran liuxiran force-pushed the refactor_publish_offline_be branch from 80692e0 to 13ae49a Compare December 18, 2020 02:04
@liuxiran
Copy link
Contributor Author

Thanks for @nic-chen 's help, all test cases passed

@juzhiyuan
Copy link
Member

How about using the codes from apisix's master branch?

@nic-chen
Copy link
Member

How about using the codes from apisix's master branch?

sure,we don't need to wait now.

This was referenced Dec 20, 2020
@liuxiran
Copy link
Contributor Author

liuxiran commented Dec 20, 2020

this pr has beening splited into two prs, and will be closed after all the separated prs merged

@nic-chen
Copy link
Member

this PR should be closed now. feel free to reopen it if you need.

@nic-chen nic-chen closed this Dec 21, 2020
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.

7 participants