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

Issue/79 #14

Merged
merged 3 commits into from
Jun 26, 2018
Merged

Issue/79 #14

merged 3 commits into from
Jun 26, 2018

Conversation

loganasherjones
Copy link
Member

Note

You should accept #13 first.

This commit allows beer-garden to safely migrate from JSON config
files to YAML config files. There is one error case, when moving
the files around could cause application startup to fail. I believe
this is mostly mitigated because we perform the scary operation
last which should throw an error before catastrophic failure
occurrs.

We also guarantee that the file will be yaml. Since yaml is a
superset of JSON, the yaml parser can load JSON just fine in
the event that migrating/moving the YAML file does not complete
successfully.

This will allow us to (in the future) be able to spawn config
watchers.
This commit allows beer-garden to safely migrate from JSON config
files to YAML config files. There is one error case, when moving
the files around could cause application startup to fail. I believe
this is mostly mitigated because we perform the scary operation
last which should throw an error before catastrophic failure
occurrs.

We also guarantee that the file will be yaml. Since yaml is a
superset of JSON, the yaml parser can load JSON just fine in
the event that migrating/moving the YAML file does not complete
successfully.
Copy link
Contributor

@hazmat345 hazmat345 left a comment

Choose a reason for hiding this comment

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

Looks like tests are failing on Python 2 😢

It may be good to throw a logging statement in _safe_migrate just so there's an explanation for why their config file magically switched from json to yaml.

@loganasherjones
Copy link
Member Author

Yeah, I didn't realize python2 was broken. I'll fix the tests later.

@loganasherjones
Copy link
Member Author

loganasherjones commented Jun 26, 2018

The python 2.7 tests are failing because of loganasherjones/yapconf#82

I will fix that and release tonight.

This required an update of yapconf which was not correctly
dumping unicode key value pairs.
@codecov
Copy link

codecov bot commented Jun 26, 2018

Codecov Report

Merging #14 into develop will increase coverage by 0.31%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #14      +/-   ##
===========================================
+ Coverage    92.34%   92.66%   +0.31%     
===========================================
  Files            9        9              
  Lines          601      627      +26     
===========================================
+ Hits           555      581      +26     
  Misses          46       46
Impacted Files Coverage Δ
bg_utils/__init__.py 100% <100%> (ø) ⬆️

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 b34c7e3...4ea211d. Read the comment docs.

@hazmat345 hazmat345 merged commit 86a23ad into develop Jun 26, 2018
@hazmat345 hazmat345 deleted the issue/79 branch June 26, 2018 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants