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: add consul discovery module #8380

Merged
merged 29 commits into from
Dec 7, 2022

Conversation

Fabriceli
Copy link
Contributor

@Fabriceli Fabriceli commented Nov 23, 2022

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
Copy link
Member

Please make the CI pass, thanks!

@Fabriceli
Copy link
Contributor Author

Please make the CI pass, thanks!

ok, i fixed it

@Fabriceli
Copy link
Contributor Author

Learn more.

Could you start the other pipeline?

@spacewander
Copy link
Member

Learn more.

Could you start the other pipeline?

Done

@tzssangglass
Copy link
Member

pls fix doc lint

@Fabriceli
Copy link
Contributor Author

pls fix doc lint

done

@Fabriceli
Copy link
Contributor Author

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

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

@spacewander @tzssangglass Updated, please check again, thanks

@Fabriceli
Copy link
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
Copy link
Contributor Author

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

@spacewander
Copy link
Member

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

@Fabriceli
Copy link
Contributor Author

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

DONE, I had merge upstream master to dev branch.

@Fabriceli
Copy link
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
Copy link
Contributor Author

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

t/discovery/consul.t Outdated Show resolved Hide resolved
@Fabriceli
Copy link
Contributor Author

Fabriceli commented Dec 6, 2022

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
Copy link
Member

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

rerun it

@Fabriceli
Copy link
Contributor Author

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

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.

3 participants