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

Format the listeners API fields #8571

Merged
merged 6 commits into from Jul 27, 2022
Merged

Conversation

HJianBo
Copy link
Member

@HJianBo HJianBo commented Jul 26, 2022

There are some an incompatible changes, but we had to fix the incorrect API formatting as soon as possible. A summary changes:

  1. Using the standard status and node_status data format for GET /listeners and GET /gateway/<name>/listeners.

    Before this changes, the MQTT listener APIs returned these fields as:

    "status": {
        "current_connections": 0,
        "max_connections": 4096000
     },
    "node_status": {
        "emqx@emqx-core-2.emqx-headless.default.svc.cluster.local": {
          "current_connections": 0,
          "max_connections": 512000
         }
    }

    The Gateway listener API returned as:

    "node_status": [
       { "current_connections": 0,
          "max_connections": 1024000,
          "node": "emqx@emqx-core-2.emqx-headless.default.svc.cluster.local"
        },
     ]
    // No `status` field

    In this PR, we unify these fields both listeners API as:

     "status": {
       "current_connections": 0,
       "max_connections": 102400,
       "running": true
     },
     "node_status": [ {
       "node": "emqx@127.0.0.1",
       "status": {
         "current_connections": 0,
         "max_connections": 102400,
         "running": true}
      }],
  2. Ensure the bind output style

    • Configured as 1883, printed as :1883
    • Configured as 0.0.0.0:1883, printed as :1883
    • Configured as 127.0.0.1:1883, printed as 127.0.0.1:1883
    • Configured as ::1:1883, printed as [::1]:1883
    • Configured as [::1]:1883, printed as [::1]:1883
  3. Move running field into status and node_status field for Gateway listeners API

  4. Add type, name fields for MQTT listeners API

@HJianBo HJianBo requested a review from a team July 26, 2022 04:01
@coveralls
Copy link
Collaborator

coveralls commented Jul 26, 2022

Pull Request Test Coverage Report for Build 2738981724

  • 29 of 41 (70.73%) changed or added relevant lines in 6 files are covered.
  • 43 unchanged lines in 19 files lost coverage.
  • Overall coverage increased (+0.02%) to 78.781%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/emqx/src/emqx_listeners.erl 7 10 70.0%
apps/emqx_gateway/src/emqx_gateway_api_listeners.erl 15 19 78.95%
apps/emqx_management/src/emqx_mgmt_api_listeners.erl 3 8 37.5%
Files with Coverage Reduction New Missed Lines %
apps/emqx_conf/src/emqx_conf_schema.erl 1 92.31%
apps/emqx_gateway/src/mqttsn/emqx_sn_channel.erl 1 72.92%
apps/emqx_management/src/emqx_mgmt_api_listeners.erl 1 73.25%
apps/emqx_management/src/emqx_mgmt_util.erl 1 13.79%
apps/emqx_slow_subs/src/emqx_slow_subs.erl 1 77.42%
apps/emqx/src/emqx_authentication_config.erl 1 84.75%
apps/emqx/src/emqx_broker.erl 1 81.87%
apps/emqx/src/emqx_channel.erl 1 85.85%
apps/emqx/src/emqx_config_handler.erl 1 96.34%
apps/emqx/src/emqx_listeners.erl 1 82.96%
Totals Coverage Status
Change from base Build 2736432390: 0.02%
Covered Lines: 20855
Relevant Lines: 26472

💛 - Coveralls

e.g:
- Configured as `1883`, printed as `:1883`
- Configured as `0.0.0.0:1883`, printed as `:1883`
- Configured as `127.0.0.1:1883`, printed as `127.0.0.1:1883`
- Configured as `::1:1883`, printed as `[::1]:1883`
- Configured as `[::1]:1883`, printed as `[::1]:1883`
@HJianBo HJianBo merged commit ec9cf0c into emqx:release-v5.0.4 Jul 27, 2022
@HJianBo HJianBo deleted the listener-apis branch July 27, 2022 01:48
HJianBo added a commit that referenced this pull request Aug 19, 2022
HJianBo added a commit to HJianBo/emqx that referenced this pull request Sep 26, 2022
In the emqx#8571, we add a colon before the
port. But it was not popular, so we removed ta in this PR
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.

None yet

4 participants