Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Return status in check mode #192

Merged
merged 4 commits into from
Sep 29, 2020
Merged

Return status in check mode #192

merged 4 commits into from
Sep 29, 2020

Conversation

Geethree
Copy link
Contributor

SUMMARY

In check mode, it is useful to know both what the current values of the release are, and the proposed values of the release based on the state of the playbook.

Addresses : #188

ISSUE TYPE
  • Feature Pull Request
Questions
  • current and proposed feel a little off in terms of ansible norms, any suggestions for something "better"?
  • What would be the best way to address this in the documentation? Maybe adding another values('check_mode') key?

It is useful to return both the current values of the release, as well as the proposed values.
@Geethree
Copy link
Contributor Author

Hmm. I'll figure out how to get these test to run locally here in a day or two.

@tima tima added has_issue This PR has a related issue it could close. type/enhancement New feature or request labels Aug 18, 2020
@Akasurde
Copy link
Member

recheck

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #192 into main will decrease coverage by 5.16%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   41.90%   36.74%   -5.17%     
==========================================
  Files           3        3              
  Lines         556      724     +168     
  Branches      114      144      +30     
==========================================
+ Hits          233      266      +33     
- Misses        279      409     +130     
- Partials       44       49       +5     
Impacted Files Coverage Δ
...ommunity/kubernetes/plugins/module_utils/common.py 35.56% <0.00%> (-2.64%) ⬇️
...s/community/kubernetes/plugins/module_utils/raw.py
...ns/community/kubernetes/plugins/action/k8s_info.py 21.83% <0.00%> (ø)
...ctions/community/kubernetes/plugins/modules/k8s.py 97.77% <0.00%> (+7.77%) ⬆️

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 4f33ba7...f88e95a. Read the comment docs.

@tima
Copy link
Collaborator

tima commented Sep 22, 2020

recheck

@geerlingguy
Copy link
Collaborator

geerlingguy commented Sep 22, 2020

@tima heh... that doesn't work in this repo. I'll rerun the tests.

Edit: For some reason, I'm not able to re-run the checks.

@tima
Copy link
Collaborator

tima commented Sep 22, 2020

@geerlingguy: bah! and i thought i was so smart.

plugins/modules/helm.py Outdated Show resolved Hide resolved
plugins/modules/helm.py Outdated Show resolved Hide resolved
plugins/modules/helm.py Outdated Show resolved Hide resolved
@fabianvf
Copy link
Collaborator

@geerlingguy got tests passing

@tima
Copy link
Collaborator

tima commented Sep 27, 2020

@Geethree:

current and proposed feel a little off in terms of ansible norms, any suggestions for something "better"?

Instead of "proposed" what about "desired" or "declared"? Those are terms we use a lot while talking about Ansible and idempotency. We use "current state" a lot though so that one seems fine to me.

plugins/modules/helm.py Outdated Show resolved Hide resolved
@Geethree
Copy link
Contributor Author

Hello. Apologies for the delay in response, and thank you to @fabianvf for finding the failure there.

I kind of like your suggestion of "proposed" @tima. Would you like to do so @fabianvf or I could step in as well? Just want to make sure we do not step on one other's toes.

@tima
Copy link
Collaborator

tima commented Sep 28, 2020

Actually, @Geethree I suggested "declared" instead of "proposed".

plugins/modules/helm.py Outdated Show resolved Hide resolved
@geerlingguy
Copy link
Collaborator

lgtm

@tima tima merged commit 7946b39 into ansible-collections:main Sep 29, 2020
@Geethree Geethree deleted the g3-helm-check-mode-values branch September 29, 2020 18:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has_issue This PR has a related issue it could close. type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants