-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(standalone): support JSON format #12333
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(standalone): support JSON format #12333
Conversation
…t-json-in-standalone-mode
…alone-mode' into young/feat/support-json-in-standalone-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mainly checked the Lua code, it seems good to me
pls fix one minor issue
I'm just saying these CSS styles are making my eyes hurt. If I had time, I'd rather rebuild the APISIX website docs part from scratch to solve a lot of the existing problems. |
Co-authored-by: Traky Deng <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends the standalone file mode to support JSON as an alternative configuration format, in addition to the existing YAML support. The changes include adding new JSON-based test cases, updating configuration file parsing logic (in apisix/core/config_yaml.lua), and adjusting CLI schema and file resolution to incorporate the new "json" provider.
- Introduce tests for JSON configurations (stream-route, ssl, route, plugin, etc.)
- Modify configuration file reading and parsing logic to support the "json" file type.
- Update CLI schema and file handling to reflect the new "json" config provider.
Reviewed Changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
t/config-center-json/*.t | Added new test cases based on JSON configuration format |
APISIX.pm | Updated file resolution to include JSON configuration files |
apisix/core/config_yaml.lua | Extended config parsing to handle JSON and YAML, using file_type switch |
apisix/core.lua | Adjusted config module loading to set file_type based on provider ("json") |
apisix/cli/schema.lua, apisix/cli/file.lua | Updated to include "json" in the acceptable config_provider enums |
Comments suppressed due to low confidence (1)
apisix/core.lua:32
- Add a clarifying comment explaining why 'file_type' is being set to 'json' here, as this behavior influences the config parsing logic. This will help future maintainers understand the conditions for JSON configuration selection.
config.file_type = "json"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good
Description
What
Support JSON as configuration format in standalone file mode
How
Extend
apisix/core/config_yaml.lua
.Therefore, without modifying
config_provider: yaml
, it is compatible with the JSON format.While this is a bit weird at the moment, I think we can change
yaml
tofile
or other more appropriate words in the future, rather than providing a new option.Because as you can see from the code, the only difference is how to read different configuration files. The current method helps us support other different formats in the future, such as
toml
or other file formats that users prefer.Why
Yaml parsing is much slower than JSON.
Test Cases
All cases are copy and changed from
t/config-center-yaml/*.t
.Which issue(s) this PR fixes:
Fixes #
Checklist