Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][WIP] Adding is_configured property #9478

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mnadeem92
Copy link
Contributor

No description provided.

@mnadeem92 mnadeem92 changed the title [WIPTEST]Adding is_configured property [NOTEST]Adding is_configured property Oct 14, 2019
@mnadeem92 mnadeem92 changed the title [NOTEST]Adding is_configured property [RFR]Adding is_configured property Oct 14, 2019
Copy link
Contributor

@john-dupuy john-dupuy left a comment

Choose a reason for hiding this comment

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

One question, thanks for this PR!

@property
def is_configured(self):
""" Check if the appliance is configured or unconfigured."""
return self.db.has_database
Copy link
Contributor

Choose a reason for hiding this comment

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

If the DB is configured shouldn't we also check if the DB has_tables

Suggested change
return self.db.has_database
return self.db.has_database and self.db.has_tables

Copy link
Contributor

@digitronik digitronik left a comment

Choose a reason for hiding this comment

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

Related to wait_for_web_ui argument in reboot

@@ -1624,7 +1629,8 @@ def restart_evm_rude(self, log_callback=None):
self.evmserverd.start()

@logger_wrap("Rebooting Appliance: {}")
def reboot(self, wait_for_web_ui=True, log_callback=None):
def reboot(self, log_callback=None):
Copy link
Contributor

@digitronik digitronik Oct 14, 2019

Choose a reason for hiding this comment

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

Required:
It will break our automation with unexpected keyword argument.
Please take wait_for_web_ui arg from user only else I would expect all changes in repo in same PR

@digitronik digitronik changed the title [RFR]Adding is_configured property [WIP] Adding is_configured property Oct 14, 2019
@dajoRH dajoRH added the WIP label Oct 14, 2019
@mnadeem92 mnadeem92 force-pushed the Add_is_configured_prop branch from 402a56f to 178c1fe Compare October 14, 2019 16:21
Modified the reboot method

Signed-off-by: mnadeem92 <[email protected]>
@mnadeem92 mnadeem92 force-pushed the Add_is_configured_prop branch from 178c1fe to 7d3df87 Compare October 14, 2019 16:29
@mnadeem92 mnadeem92 changed the title [WIP] Adding is_configured property [RFR] Adding is_configured property Oct 14, 2019
Copy link
Contributor

@digitronik digitronik left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@digitronik digitronik changed the title [RFR] Adding is_configured property [1LP][RFR] Adding is_configured property Oct 14, 2019
@dajoRH dajoRH removed the WIP label Oct 14, 2019
@property
def is_configured(self):
""" Check if the appliance is configured or unconfigured."""
return self.db.has_database and self.db.has_tables
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd look for better way to check whether appliance configured or not. existence of database and tables doesn't guarantee that appliance is configured.

@izapolsk izapolsk changed the title [1LP][RFR] Adding is_configured property [1LP][WIP] Adding is_configured property Oct 18, 2019
@izapolsk izapolsk self-assigned this Oct 18, 2019
@dajoRH dajoRH added the WIP label Oct 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants