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

refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs #8661

Merged
merged 7 commits into from
Jan 31, 2023

Conversation

An-DJ
Copy link
Contributor

@An-DJ An-DJ commented Jan 12, 2023

Description

Fixes #8569

  • stream_routes
  • upstreams
  • protos
  • services
  • global_rules
  • consumer_groups
  • plugin_configs

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)

@An-DJ An-DJ changed the title [WIP]refactor(admin): stream_routes refactor(admin): stream_routes [WIP] Jan 12, 2023
@An-DJ An-DJ changed the title refactor(admin): stream_routes [WIP] refactor(admin): stream_routes/upstreams [WIP] Jan 12, 2023
@An-DJ An-DJ changed the title refactor(admin): stream_routes/upstreams [WIP] refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs [WIP] Jan 12, 2023
@An-DJ An-DJ changed the title refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs [WIP] refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs Jan 12, 2023

utils.fix_count(res.body, id)
return res.status, res.body
return need_id and id or true
Copy link
Member

Choose a reason for hiding this comment

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

Look like we can move this line to resource.lua?

@@ -143,5 +143,6 @@ return resource.new({
name = "routes",
kind = "route",
schema = core.schema.route,
checker = check_conf
checker = check_conf,
unsupported_methods = {}
Copy link
Member

Choose a reason for hiding this comment

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

We can leave this nil as this resource doesn't have unsupported methods.

kind = "service",
schema = core.schema.service,
checker = check_conf,
unsupported_methods = {},
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

return self.checker(id, conf, need_id, self.schema)
local ok, err = self.checker(id, conf, need_id, self.schema)

if err then
Copy link
Member

Choose a reason for hiding this comment

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

Code style, please use if not ok then

spacewander
spacewander previously approved these changes Jan 17, 2023
@@ -199,7 +199,17 @@ local function run()
end

local code, data
if seg_res == "routes" then
local refactored_resources = {
Copy link
Contributor

Choose a reason for hiding this comment

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

directly named as resources?

Copy link
Contributor Author

@An-DJ An-DJ Jan 17, 2023

Choose a reason for hiding this comment

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

There has been a global variable named resources already. 😭 (https://github.com/apache/apisix/blob/master/apisix/admin/init.lua#L46)

Maybe we can rename that variable after all resources are refactored.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

end


function _M:get(id)
if self.unsupported_methods and core.table.array_find(self.unsupported_methods, "get") then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it enough to use if core.table.array_find(self.unsupported_methods, "get") then ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@soulbird Done.

@spacewander spacewander merged commit 22be6d0 into apache:master Jan 31, 2023
hongbinhsu added a commit to fitphp/apix that referenced this pull request Feb 3, 2023
* upstream/master:
  feat(elasticsearch-logger): support multi elasticsearch endpoints (apache#8604)
  chore: use operator # instead of string.len (apache#8751)
  chore: hi 2023 (apache#8748)
  refactor(admin): stream_routes/upstreams/protos/services/global_rules/consumer_groups/plugin_configs (apache#8661)
  feat: support send error-log to kafka brokers (apache#8693)
  chore: upgrade `casbin` to `1.41.5` (apache#8744)
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.

feat: As a user, I want to refactor the resources under apisix.admin, so that we don't need to repeat
3 participants