Skip to content
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

Add status queries to the skill process #2304

Merged
merged 1 commit into from Sep 14, 2019

Conversation

@forslund
Copy link
Member

commented Sep 13, 2019

Description

mycroft.skills.is_alive: The service is started and priority skills are
loaded.
mycroft.skills.all_loaded: All skills on the system has been loaded.

How to test

Check the return of self.bus.wait_for_response(Message('mycroft.skills.is_alive')) and self.bus.wait_for_response(Message('mycroft.skills.all_loaded')) from a skill.

Contributor license agreement signed?

CLA [ Yes ]

@forslund forslund requested a review from davidwagnerkc Sep 13, 2019
@forslund

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2019

An alternative could be to have single mycroft.skills.status returning a data structure

{
  "priority_loaded": true,
  "all_loaded": true
}
mycroft.skills.is_alive: The service is started and priority skills are
loaded.
mycroft.skills.all_loaded: All skills on the system has been loaded.
@forslund forslund force-pushed the forslund:feature/skill-status-queries branch from 2a3ef9f to 7346e4d Sep 13, 2019
Copy link
Contributor

left a comment

Works as advertised 👍 Maybe we could move to your suggested mycroft.skills.status if we end up having more state like this that might need to be queried. For now with just these 2 key confirmations I think this is good.

@forslund

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2019

Agreed. Merging

@forslund forslund merged commit accafee into MycroftAI:dev Sep 14, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.003%) to 53.243%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.