Issue-3100 - Adding skills methods for padatious

[zeronounours]

Description

Add two new skills methods to be able to load any file as padatious
intent or entity, even outside of vocab and locale directories.

Fix #3100

How to test

I also added unittests on test_mycroft_skill.py

Contributor license agreement signed?

CLA [x] (Whether you have signed a CLA - Contributor Licensing Agreement

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[krisgesling]

Hey there, thanks for helping to improve Mycroft!

I can definitely see the appeal here. I'm tossing up in my mind whether a separate method is necessary or if we should check to see if the intent_file passed into the existing method is a full path?

• In one sense there is simplicity in having a single method to register intents and a single method to register entities.
• In another sense it adds complexity to that method since it can now accept more variations.
• Creating the new method also reduces chances of unexpected behaviours being introduced for existing Skills.

Interested in any opinions on this.

If we do go with a separate method, I'm wondering if we can find a more descriptive name. If I'm developing a Skill and see:

• register_intent_file()
• register_intent_file_as()

It's not immediately clear to me how they differ. I might also interpret the new method as if I'm going to register an intent file as something else. Would something like register_intent_file_path() provide a better distinction?

Edit: Just fyi I doubt the VK test failure reported above is related to this PR.

[zeronounours]

The current methods allows the use of full path because of find_resource relying on os.path.join which allows full path components. However, the name of the intent is based on that path and is not sanitized.

Home Assistant skill recently started using full path, but it fails in the end (see MycroftAI/skill-homeassistant#104) as padatious fails to save the network file:

FANN Error 2: Unable to open configuration file "/opt/mycroft/.local/share/mycroft/intent_cache/skill-homeassistant.mycroftai:{/tmp/mycroft/cache/HomeAssistantSkill/tracker}.intent.net" for writing.

Another solution may be to sanitize the path when generating the name to only keep the basename. However, I think some developpers will have trouble understanding why they fails to load my_entity.entity and /tmp/path/my_entity.entity.

The name register_intent_file_path() is fine for me too.

[zeronounours]

I force-pushed a new version with the proposed names register_intent_file_path and register_entity_file_path.

[forslund]

Closing PR since we're archiving the repo