Skip to content

Default Config #35

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

Merged
merged 1 commit into from
Aug 27, 2017
Merged

Default Config #35

merged 1 commit into from
Aug 27, 2017

Conversation

gronke
Copy link
Member

@gronke gronke commented Aug 27, 2017

implements #26

  • load default configuration from default.json
  • assumes default.json is stored in zpool <ACTIVE_POOL>/iocage
  • can modify default.json

Additional Changes

  • cleaned up list of system defaults
  • more comments

@gronke gronke force-pushed the feature/default-config branch 2 times, most recently from 5ce788f to 0ff3cad Compare August 27, 2017 16:54
@@ -51,6 +51,7 @@ def cli(ctx, rc, jails, log_level):
try:
jail.start()
except Exception:
raise
Copy link
Contributor

Choose a reason for hiding this comment

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

exit(1) will never be reached.

# if self.data["jail_zfs"] does not explicitly exist, _get_jail_zfs
# would raise
# if self.data["jail_zfs"] does not explicitly exist,
# _get_jail_zfs would raise
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this a docstring, two comments at the top look ugly

except:
pass

# if it was not, the default for is 'nullfs' if the jail is a basejail
Copy link
Contributor

Choose a reason for hiding this comment

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

If it was not, the default for a basejail type is nullfs reads better

@@ -24,7 +24,9 @@ def read(self):

def read_data(self):
with open(JailConfigJSON.__get_config_json_path(self), "r") as conf:
return json.load(conf)
data = json.load(conf)
conf.close()
Copy link
Contributor

Choose a reason for hiding this comment

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

The with statement already closes once you leave it, that shouldn't be necessary.

def __init__(self, config_type, *args, **kwargs):
msg = f"Could not read {config_type} config"
# This is a silent error internally used
Exception.__init__(self, msg, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't correct, __init__ only accepts *args and **kwargs. If we ever used this internally, it wouldn't function.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a problem here - right. But we're seeing different ones here. I would have removed *args, **kwargs from Exception.init, because we only want to raise this message and nothing else.

@gronke gronke force-pushed the feature/default-config branch 2 times, most recently from a737e39 to 9d36222 Compare August 27, 2017 18:13
skarekrow
skarekrow previously approved these changes Aug 27, 2017
skarekrow
skarekrow previously approved these changes Aug 27, 2017
@gronke gronke force-pushed the feature/default-config branch from e1f4a91 to bcc75c7 Compare August 27, 2017 18:43
@gronke gronke merged commit 7f0e47c into master Aug 27, 2017
@gronke gronke deleted the feature/default-config branch August 27, 2017 19:00
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