Skip to content

Add Facebox auth#5789

Merged
MartinHjelmare merged 4 commits intonextfrom
robmarkcole-patch-1
Aug 8, 2018
Merged

Add Facebox auth#5789
MartinHjelmare merged 4 commits intonextfrom
robmarkcole-patch-1

Conversation

@robmarkcole
Copy link
Copy Markdown
Contributor

@robmarkcole robmarkcole commented Jul 14, 2018

Description:
Adds Facebox auth and removes teach events

Pull request in home-assistant (if applicable): home-assistant/core#15439

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the standards.

Adds auth and removes teach events
@ghost ghost assigned robmarkcole Jul 14, 2018
@ghost ghost added the ready-for-review This PR needs to be reviewed label Jul 14, 2018
@robmarkcole robmarkcole requested a review from arsaboo July 14, 2018 05:33
@frenck frenck added next This PR goes into the next branch has-parent This PR has a parent PR in another repo new-feature This PR adds documentation for a new Home Assistant feature to an existing integration labels Jul 14, 2018
```
{% endraw %}

An `image_processing.teach_classifier` event is fired for each service call, providing feedback on whether teaching has been successful or unsuccessful. In the unsuccessful case, the `message` field of the `event_data` will contain information on the cause of failure, and a warning is also published in the logs. An automation can be used to receive alerts on teaching, for example, the following automation will send a notification with the teaching image and a message describing the status of the teaching:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this service being removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, in the latest PR

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.

The service isn't removed but the event fired on service call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry yes thats correct

arsaboo
arsaboo previously approved these changes Jul 14, 2018
Copy link
Copy Markdown
Contributor

@arsaboo arsaboo left a comment

Choose a reason for hiding this comment

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

Looks good..can be merged when the parent PR is merged.

@arsaboo arsaboo added awaits-parent Awaits the merge of an parent PR and removed ready-for-review This PR needs to be reviewed labels Jul 14, 2018
@MartinHjelmare MartinHjelmare changed the title Adds Facebox auth Add Facebox auth Aug 7, 2018
@robmarkcole
Copy link
Copy Markdown
Contributor Author

@arsaboo parent PR has been merged now


sudo docker run --name=facebox --restart=always -p 8080:8080 -e "MB_KEY=$MB_KEY" machinebox/facebox
```
If you are on an unsecured network you can run Facebox with a username and password, add ` -e "MB_BASICAUTH_USER=my_username" -e "MB_BASICAUTH_PASS=my_password" `
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.

The requests are still not encrypted right? So we shouldn't recommend this on an unsecured network?

Copy link
Copy Markdown
Contributor Author

@robmarkcole robmarkcole Aug 7, 2018

Choose a reason for hiding this comment

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

I hadn't considered that, but you are correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess the statement should be: 'You can run Facebox .. but bare in mind that the component does not encrypt these credentials'?


sudo docker run --name=facebox --restart=always -p 8080:8080 -e "MB_KEY=$MB_KEY" machinebox/facebox
```
You can run Facebox with a username and password by adding ` -e "MB_BASICAUTH_USER=my_username" -e "MB_BASICAUTH_PASS=my_password" ` but bare in mind that the component does not encrypt these credentials and this approach does not guarantee security on an unsecured network.
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.

The formatting of the literal text within back-tics looks weird in the rendered page. Check the netlify preview.

@robmarkcole
Copy link
Copy Markdown
Contributor Author

ping @MartinHjelmare

@MartinHjelmare MartinHjelmare merged commit a406390 into next Aug 8, 2018
@ghost ghost removed the awaits-parent Awaits the merge of an parent PR label Aug 8, 2018
dbrowndan pushed a commit to dbrowndan/home-assistant.io that referenced this pull request Aug 8, 2018
* Adds auth

Adds auth and removes teach events

* Update image_processing.facebox.markdown

* Update image_processing.facebox.markdown

* Remove whitespace around `
@frenck frenck deleted the robmarkcole-patch-1 branch August 13, 2018 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-parent This PR has a parent PR in another repo new-feature This PR adds documentation for a new Home Assistant feature to an existing integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants