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

'configure show changed' includes deleted elements #470

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

liangxin1300
Copy link
Collaborator

@liangxin1300 liangxin1300 commented Aug 27, 2019

Problem:
Our user complain that after delete resource(before commit), 'crm configure show changed' not list the elements just deleted.

Working with crm in SLES12 SP4 I noticed that when you delete resources in "crm configure", a > "show changed" just displays nothing.
How hard would it be to show commands like "delete xyz" for each deleted resource?

Solution:
So I append this line:

obj_set |= orderedset.oset(self.remove_queue)

in filter_objects function, cibconfig.py file.

Test:
I also append lines in test/testcases/commit:

delete d3
show changed

to test this scenarios.
Behave seems not suitable to test interactive scenarios.

@liangxin1300 liangxin1300 changed the title 20190827 show changed (WIP)20190827 show changed Aug 27, 2019
@liangxin1300 liangxin1300 force-pushed the 20190827_show_changed branch 4 times, most recently from ca626ad to d1b37ed Compare August 27, 2019 08:32
@liangxin1300 liangxin1300 changed the title (WIP)20190827 show changed 'configure show changed' includes deleted elements Aug 27, 2019
@liangxin1300 liangxin1300 force-pushed the 20190827_show_changed branch 5 times, most recently from a830988 to bf4f62d Compare December 10, 2019 06:11
@liangxin1300 liangxin1300 changed the title 'configure show changed' includes deleted elements (WIP)'configure show changed' includes deleted elements Dec 10, 2019
@liangxin1300 liangxin1300 force-pushed the 20190827_show_changed branch 2 times, most recently from 4afbcd1 to a914ce7 Compare December 10, 2019 07:40
@liangxin1300 liangxin1300 changed the title (WIP)'configure show changed' includes deleted elements 'configure show changed' includes deleted elements Dec 10, 2019
@liangxin1300 liangxin1300 force-pushed the 20190827_show_changed branch 2 times, most recently from 0305d48 to 75acf0a Compare December 10, 2019 14:24
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

@liangxin1300 I don't really understand the change.
Shouldn't be the expected behaviour that the delete command is shown after show changed?
At least in the test is not shown...
Instead we print primitive d2 Dummy. Is this what you want?

@liangxin1300
Copy link
Collaborator Author

@liangxin1300 I don't really understand the change.
Shouldn't be the expected behaviour that the delete command is shown after show changed?
At least in the test is not shown...
Instead we print primitive d2 Dummy. Is this what you want?

@arbulu89 , show changed will show both modified and deleted elements.
Current show changed doesn't include deleted elements, just show modified

@liangxin1300
Copy link
Collaborator Author

@arbulu89 maybe we could add show changed modified and show changed deleted?
I think it looks like a bit of complicated.
I think show changed(without more sub-parameters) is enough.

@arbulu89
Copy link
Contributor

@arbulu89 maybe we could add show changed modified and show changed deleted?
I think it looks like a bit of complicated.
I think show changed(without more sub-parameters) is enough.

That's OK for me, but I don't know if matches with that user expectation. He won't find any output like delete resource (what he comments). He will find resource changed what is different. Maybe, when the resource is deleted, instead of printing primitive d2 Dummy he would prefer primitive d2 Dummy deleted. But I don't know if this is something easy to do

@liangxin1300
Copy link
Collaborator Author

@arbulu89 I will collect suggestions from this user

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.

2 participants