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.

Sign up for GitHub

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

Jump to bottom

api: omit empty fields in /experiments/detail #700

Merged
merged 5 commits into from
Jul 20, 2020
Merged

api: omit empty fields in /experiments/detail #700

merged 5 commits into from
Jul 20, 2020

Conversation

g1eny0ung
Copy link
Member

@g1eny0ung g1eny0ung commented Jul 15, 2020 •
edited
Loading

What problem does this PR solve?

This PR omits the empty fields in the /experiments/detail response.

What is changed and how does it work?

For example, a NetworkChaos:

Before:

"target": {
    "kind": "",
    "pod_chaos": {
      "action": "",
      "container_name": ""
    },
    "network_chaos": {
      "action": "delay",
      "delay": {
        "latency": "30ms",
        "correlation": "0",
        "jitter": "0ms"
      },
      "loss": null,
      "duplicate": null,
      "corrupt": null,
      "bandwidth": null,
      "direction": "to",
      "target_scope": {
        "namespace_selectors": [
          "default"
        ],
        "label_selectors": {
          "app": "hello-minikube"
        },
        "annotation_selectors": null,
        "field_selectors": null,
        "phase_selectors": null,
        "pods": null,
        "mode": "",
        "value": ""
      }
    },
    "io_chaos": {
      "action": "",
      "addr": "",
      "delay": "",
      "errno": "",
      "path": "",
      "percent": "",
      "methods": null
    },
    "kernel_chaos": {
      "fail_kernel_req": {
        "failtype": 0
      }
    },
    "time_chaos": {
      "offset": "",
      "clock_ids": null,
      "container_names": null
    },
    "stress_chaos": {
      "stressors": null
    }
  }

After:

"target": {
    "kind": "NetworkChaos",
    "network_chaos": {
      "action": "delay",
      "delay": {
        "latency": "30ms",
        "correlation": "0",
        "jitter": "0ms"
      },
      "loss": null,
      "duplicate": null,
      "corrupt": null,
      "bandwidth": null,
      "direction": "to",
      "target_scope": {
        "namespace_selectors": [
          "default"
        ],
        "label_selectors": {
          "app": "hello-minikube"
        },
        "annotation_selectors": null,
        "field_selectors": null,
        "phase_selectors": null,
        "pods": null,
        "mode": "",
        "value": ""
      }
    }
  }

Check List

Code changes

  • Has Go code change

@g1eny0ung g1eny0ung requested review from fewdan and cwen0 July 15, 2020 05:30
@g1eny0ung g1eny0ung force-pushed the api/omit-experiment-detail branch from 0a29c92 to 17e7896 Compare July 15, 2020 05:34
@g1eny0ung g1eny0ung changed the title api: omit empty fields in /experiment/detail api: omit empty fields in /experiments/detail Jul 15, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2020 •
edited
Loading

Codecov Report

Merging #700 into master will decrease coverage by 1.29%.
The diff coverage is 36.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #700      +/-   ##
==========================================
- Coverage   55.78%   54.49%   -1.30%     
==========================================
  Files          68       69       +1     
  Lines        4383     4652     +269     
==========================================
+ Hits         2445     2535      +90     
- Misses       1768     1927     +159     
- Partials      170      190      +20
Impacted Files Coverage Δ
api/v1alpha1/networkchaos_types.go 22.98% <ø> (ø)
api/v1alpha1/podchaos_types.go 37.14% <ø> (ø)
controllers/networkchaos/netem/utils.go 0.00% <0.00%> (ø)
pkg/chaosdaemon/netlink_linux.go 0.00% <0.00%> (ø)
pkg/chaosdaemon/stress_server_linux.go 0.00% <0.00%> (ø)
pkg/chaosdaemon/tc_linux.go 42.85% <0.00%> (ø)
pkg/utils/chaosdaemon.go 51.21% <0.00%> (ø)
pkg/utils/grpc.go 0.00% <0.00%> (ø)
pkg/webhook/inject/inject.go 73.49% <0.00%> (-0.68%) ⬇️
pkg/chaosdaemon/util.go 72.48% <9.09%> (-10.60%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce96bb4...de6e59e. Read the comment docs.

zhouqiang-cl
zhouqiang-cl previously approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl
Copy link
Contributor

/merge

@zhouqiang-cl
Copy link
Contributor

/run-e2e-test

@fewdan
Copy link
Member

fewdan commented Jul 16, 2020

please fix CI

@g1eny0ung
fix: ci
00e605a 
Signed-off-by: Yue Yang <[email protected]>
@g1eny0ung
Copy link
Member Author

@fewdan Already fixed. Thx for mentioned!

cwen0
cwen0 reviewed Jul 17, 2020
View reviewed changes
pkg/apiserver/experiment/experiment.go
TimeChaos TimeChaosInfo `json:"time_chaos"`
StressChaos StressChaosInfo `json:"stress_chaos"`
Kind string `json:"kind" binding:"required,oneof=PodChaos NetworkChaos IoChaos KernelChaos TimeChaos StressChaos"`
PodChaos *PodChaosInfo `json:"pod_chaos,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

If we use the pointer here, we should check the nil when using them.

cwen0
cwen0 approved these changes Jul 17, 2020
View reviewed changes
Copy link
Member

@cwen0 cwen0 left a comment

Choose a reason for hiding this comment

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

LGTM

@g1eny0ung g1eny0ung mentioned this pull request Jul 17, 2020
Merged
fewdan
fewdan approved these changes Jul 20, 2020
View reviewed changes
Copy link
Member

@fewdan fewdan left a comment

Choose a reason for hiding this comment

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

LGTM

@fewdan
Copy link
Member

fewdan commented Jul 20, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8031c07 into chaos-mesh:master Jul 20, 2020
@g1eny0ung g1eny0ung deleted the api/omit-experiment-detail branch July 22, 2020 07:51
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
@g1eny0ung @ti-srebot
api: omit empty fields in /experiments/detail (chaos-mesh#700)
0b3ab2b 
* chore: omit empty fields

Signed-off-by: Yue Yang <[email protected]>

* fix: typo

Signed-off-by: Yue Yang <[email protected]>

* fix: ci

Signed-off-by: Yue Yang <[email protected]>

Co-authored-by: ti-srebot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fewdan fewdan fewdan approved these changes

@cwen0 cwen0 cwen0 approved these changes

@zhouqiang-cl zhouqiang-cl zhouqiang-cl left review comments

Assignees
No one assigned
Labels
status/can-merge status/LGT1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@g1eny0ung @codecov-commenter @zhouqiang-cl @fewdan @ti-srebot @cwen0