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 control api for discovery module #3742

Merged
merged 6 commits into from
Mar 8, 2021

Conversation

yongboy
Copy link
Contributor

@yongboy yongboy commented Mar 3, 2021

What this PR does / why we need it:

Just implement the _M.dump_data() function for discovery module, then call the control api for dumping running details online.

The dump api uri template:

GET /v1/discovery/{discovery_type}/dump

eg:

curl http://127.0.0.1:9090/v1/discovery/eureka/dump

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@spacewander
Copy link
Member

The conflict needs to be solved so that the CI can run.

docs/en/latest/discovery.md Outdated Show resolved Hide resolved
if discovery_type then
local discovery = require("apisix.discovery.init").discovery
local dump_apis = {}
for key, _ in pairs(discovery_type) do
Copy link
Member

Choose a reason for hiding this comment

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

It seems we can use for key, dis_mod in pairs(discovery) do directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if use for key, dis_mod in pairs(discovery) do, the dis_mod is function type, not a table object.

So, we can't do it.

t/control/discovery.t Outdated Show resolved Hide resolved
GET /v1/discovery/eureka/dump
--- error_code: 200
--- response_body_unlike
^{}$
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Y

@spacewander
Copy link
Member

The CI fails because of #3747

@@ -92,6 +95,11 @@ Then implement the `_M.init_worker()` function for initialization and the `_M.no
end


function _M.dump_data()
... ...
Copy link
Contributor

Choose a reason for hiding this comment

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

I think an example return data shoul be written here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for example ?

  function _M.dump_data()
      return {config = your_config, services = your_services, other = ... } 
  end

@spacewander spacewander merged commit 34f60ee into apache:master Mar 8, 2021
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