Skip to content

Check if a script requirement is available before install#20517

Merged
balloob merged 4 commits into
home-assistant:devfrom
kellerza:script_req
Feb 26, 2019
Merged

Check if a script requirement is available before install#20517
balloob merged 4 commits into
home-assistant:devfrom
kellerza:script_req

Conversation

@kellerza
Copy link
Copy Markdown
Member

@kellerza kellerza commented Jan 27, 2019

Description:

Retry of #20272

  • Current behaviour always installs a script's requirement

  • This checks if a requirement's project name is available (by import) before it forces install

There is full (including version) requirement handling available in requirements.py, but this requires a full hass instance and seems like an overkill for scripts

Related issue (if applicable): fixes #18651

hass --script check_config
INFO:homeassistant.util.package:Attempting install of colorlog==4.0.2

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

@homeassistant homeassistant added cla-signed core small-pr PRs with less than 30 lines. labels Jan 27, 2019
@ghost ghost assigned kellerza Jan 27, 2019
@ghost ghost added the in progress label Jan 27, 2019
Comment thread homeassistant/scripts/__init__.py Outdated
for req in getattr(script, 'REQUIREMENTS', []):
try:
# Only use the requirement's project_name to verify it exists
project_name = Requirement.parse(req).project_name
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's not reinvent the wheel. Import PackageLoadable from homeassistant.requirements

Comment thread homeassistant/scripts/__init__.py Outdated
from homeassistant.exceptions import HomeAssistantError

REQUIREMENTS = ('colorlog==4.0.2',)
if system() == 'Windows': # Ensure colorama installed for colorlog on Windows
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this no longer necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It’s automatic these days. In the past you needed colorlog[windows]

See https://pypi.org/project/colorlog/ search windows/colorama

@balloob balloob merged commit 90d3f51 into home-assistant:dev Feb 26, 2019
@ghost ghost removed the in progress label Feb 26, 2019
@balloob balloob mentioned this pull request Mar 6, 2019
@awarecan awarecan mentioned this pull request Mar 7, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed core small-pr PRs with less than 30 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trying to install color-log every time config check is run in CLI

4 participants