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

Modules/NM: Wrap changes in checkpoint #119

Merged
merged 2 commits into from
Jun 14, 2019
Merged

Modules/NM: Wrap changes in checkpoint #119

merged 2 commits into from
Jun 14, 2019

Conversation

tyll
Copy link
Member

@tyll tyll commented Jun 12, 2019 •
edited
Loading

Create a checkpoint before changing NetworkManager profiles and rollback
on failures (destroy it on success).

This fixes #33

Deferred integration tests, since the code is covered by the other tests. only the rollback is not tested.

This change is Reviewable

@tyll tyll requested a review from thom311 June 12, 2019 19:21
@coveralls
Copy link

coveralls commented Jun 12, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 321

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 555 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.4%) to 38.207%
Files with Coverage Reduction New Missed Lines %
/home/travis/build/linux-system-roles/network/module_utils/network_lsr/utils.py 44 53.71%
/home/travis/build/linux-system-roles/network/library/network_connections.py 511 19.38%
Totals Coverage Status
Change from base Build 294: -0.4%
Covered Lines: 797
Relevant Lines: 2086
💛 - Coveralls

@tyll
Copy link
Member Author

tyll commented Jun 12, 2019

[citest bad]

thom311
thom311 reviewed Jun 13, 2019
View reviewed changes
library/network_connections.py Outdated Show resolved Hide resolved
library/network_connections.py Outdated Show resolved Hide resolved
library/network_connections.py Outdated Show resolved Hide resolved
library/network_connections.py Outdated Show resolved Hide resolved
@tyll tyll force-pushed the checkpoints branch 2 times, most recently from 024e840 to 4dce460 Compare June 13, 2019 07:13
@tyll tyll force-pushed the checkpoints branch 3 times, most recently from 5858e95 to 8ee53ba Compare June 13, 2019 20:42
@tyll tyll mentioned this pull request Jun 13, 2019
Merged
@thom311
Copy link
Contributor

thom311 commented Jun 14, 2019

lgtm

tyll added 2 commits June 14, 2019 12:00
@tyll
Utils: Add helper to call sync NM methods
c443947
@tyll
Modules/NM: Wrap changes in checkpoint
225ba70 
Create a checkpoint before changing NetworkManager profiles and rollback
on failures (destroy it on success).
@tyll
Copy link
Member Author

tyll commented Jun 14, 2019

[citest pending]

1 similar comment
@tyll
Copy link
Member Author

tyll commented Jun 14, 2019

[citest pending]

@tyll
Copy link
Member Author

tyll commented Jun 14, 2019

[citest bad]

@tyll tyll merged commit e37e9d7 into linux-system-roles:master Jun 14, 2019
@tyll tyll deleted the checkpoints branch June 14, 2019 13:09
This was referenced Feb 20, 2020
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thom311 thom311 thom311 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFE] use NetworkManager's Checkpoint feature to rollback on error
3 participants
@tyll @coveralls @thom311