Skip to content

Polymer access to log file broken when using new log file command line#9437

Merged
balloob merged 3 commits into
home-assistant:devfrom
TD22057:log_file
Sep 16, 2017
Merged

Polymer access to log file broken when using new log file command line#9437
balloob merged 3 commits into
home-assistant:devfrom
TD22057:log_file

Conversation

@TD22057
Copy link
Copy Markdown
Contributor

@TD22057 TD22057 commented Sep 15, 2017

Description:

If the new command line input for the log file is used from PR #9422, the polymer developer tools info page cannot find the log file location. It uses the default log file location and the hass config path to load a file that is no longer there. This PR fixes that issue by creating a new variable ERROR_LOG_PATH which components/api.py can access to load the actual log file location.

FYI I could not run tox. The current state of the dev branch throws the following error. Manual tests were run w/ the fix and the webui finds the default or custom log file correctly now.

tests/components/cloud/test_auth_api.py:4: in <module>
    from botocore.exceptions import ClientError
E   ImportError: No module named 'botocore'

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@TD22057, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @JshWright and @pvizeli to be potential reviewers.

Comment thread homeassistant/bootstrap.py Outdated
pass

# Log errors to a file if we have write access to file or config dir
global ERROR_LOG_PATH
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.

Please store data in hass.data, a dictionary for this purpose. We should not use globals.

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 15, 2017

Tox does not re-create the test environment on each invocation. Since our test requirements changed, you need to re-create it. Run once: tox -r

@TD22057
Copy link
Copy Markdown
Contributor Author

TD22057 commented Sep 15, 2017

Fixed. Thanks for the help. I tried to check the key I selected to make sure it wasn't in use somewhere else but that proved to be difficult.

@balloob balloob merged commit 26c9851 into home-assistant:dev Sep 16, 2017
@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 16, 2017

Awesome! 🐬 🌮 Thanks!

@balloob balloob mentioned this pull request Sep 22, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 2018
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