Skip to content

feat: add consul discovery module#8380

Merged
spacewander merged 29 commits into
apache:masterfrom
Fabriceli:feat/add_consul_discovery
Dec 7, 2022
Merged

feat: add consul discovery module#8380
spacewander merged 29 commits into
apache:masterfrom
Fabriceli:feat/add_consul_discovery

Conversation

@Fabriceli

@Fabriceli Fabriceli commented Nov 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

As I mentioned previously in #8371 , my team submit our consul discovery module

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@soulbird soulbird changed the title Feat: add consul discovery module feat: add consul discovery module Nov 23, 2022
@spacewander

spacewander commented Nov 24, 2022

Copy link
Copy Markdown
Member

Please make the CI pass, thanks!

@Fabriceli

Fabriceli commented Nov 24, 2022

Copy link
Copy Markdown
Contributor Author

Please make the CI pass, thanks!

ok, i fixed it

@Fabriceli

Fabriceli commented Nov 24, 2022

Copy link
Copy Markdown
Contributor Author

Learn more.

Could you start the other pipeline?

@spacewander

spacewander commented Nov 25, 2022

Copy link
Copy Markdown
Member

Learn more.

Could you start the other pipeline?

Done

tzssangglass
tzssangglass reviewed Nov 25, 2022
View reviewed changes
Comment thread docs/en/latest/discovery/consul.md Outdated
tzssangglass
tzssangglass reviewed Nov 25, 2022
View reviewed changes
Comment thread apisix/discovery/consul/init.lua Outdated
@tzssangglass

tzssangglass commented Nov 25, 2022

Copy link
Copy Markdown
Member

pls fix doc lint

@Fabriceli

Fabriceli commented Nov 25, 2022

Copy link
Copy Markdown
Contributor Author

pls fix doc lint

done

@Fabriceli

Fabriceli commented Nov 29, 2022

Copy link
Copy Markdown
Contributor Author

@spacewander @tzssangglass I had finished, and I had fixed all the CI error

@Fabriceli
Fabriceli requested review from spacewander and tzssangglass and removed request for spacewander and tzssangglass November 30, 2022 11:24
@Fabriceli

Fabriceli commented Dec 1, 2022

Copy link
Copy Markdown
Contributor Author

@spacewander @tzssangglass Updated, please check again, thanks

@Fabriceli

Fabriceli commented Dec 1, 2022

Copy link
Copy Markdown
Contributor Author

@spacewander @tzssangglass the CI with the t/xds-library/config_xds_2.t TEST 7 is not stable, I did not modified anything about that

@Fabriceli

Fabriceli commented Dec 2, 2022

Copy link
Copy Markdown
Contributor Author

@spacewander @tzssangglass May you RETURN CI to rerun the CI again, thanks

@spacewander

spacewander commented Dec 2, 2022

Copy link
Copy Markdown
Member

@Fabriceli
Could you follow the discussion in #8433 (comment)?
Thanks!

@Fabriceli

Fabriceli commented Dec 2, 2022

Copy link
Copy Markdown
Contributor Author

@Fabriceli Could you follow the discussion in #8433 (comment)? Thanks!

DONE, I had merge upstream master to dev branch.

@Fabriceli

Fabriceli commented Dec 2, 2022

Copy link
Copy Markdown
Contributor Author

@Fabriceli Could you follow the discussion in #8433 (comment)? Thanks!

@Fabriceli Fabriceli closed this Dec 2, 2022
@Fabriceli Fabriceli reopened this Dec 2, 2022
@Fabriceli

Fabriceli commented Dec 3, 2022

Copy link
Copy Markdown
Contributor Author

@spacewander @tzssangglass I have merged upstream master, could you re-start the CI? Thanks

tzssangglass
tzssangglass reviewed Dec 5, 2022
View reviewed changes
Comment thread apisix/discovery/consul/init.lua Outdated
tzssangglass
tzssangglass reviewed Dec 5, 2022
View reviewed changes
Comment thread apisix/discovery/consul/init.lua
tzssangglass
tzssangglass reviewed Dec 5, 2022
View reviewed changes
Comment thread apisix/discovery/consul/init.lua Outdated
tzssangglass
tzssangglass reviewed Dec 5, 2022
View reviewed changes
Comment thread t/discovery/consul.t Outdated
tzssangglass
tzssangglass reviewed Dec 5, 2022
View reviewed changes
Comment thread t/discovery/consul.t
@Fabriceli

Fabriceli commented Dec 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Some error in Chaos Test, May you give some hint about that? cc @spacewander @tzssangglass

Failure [0.077 seconds]
Test APISIX Delay When Add ETCD Delay
/home/runner/work/apisix/apisix/t/chaos/delayetcd/delayetcd.go:100
  get default apisix delay [It]
  /home/runner/work/apisix/apisix/t/chaos/delayetcd/delayetcd.go:133

  Expected
      <time.Duration>: 15246514
  to be <
      <time.Duration>: 15000000

  /home/runner/work/apisix/apisix/t/chaos/delayetcd/delayetcd.go:136

Error Stack: https://github.com/apache/apisix/actions/runs/3617352817/jobs/6100135416

@tzssangglass

tzssangglass commented Dec 6, 2022

Copy link
Copy Markdown
Member

Some error in Chaos Test, May you give some hint about that

rerun it

@Fabriceli

Fabriceli commented Dec 6, 2022

Copy link
Copy Markdown
Contributor Author

Please take a look at this CR, thanks, cc @spacewander

@spacewander
spacewander merged commit 04dbc61 into apache:master Dec 7, 2022
@shreemaan-abhishek shreemaan-abhishek mentioned this pull request Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacewander spacewander spacewander approved these changes
@tzssangglass tzssangglass tzssangglass approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Fabriceli @spacewander @tzssangglass