Skip to content
This repository has been archived by the owner on Oct 24, 2021. It is now read-only.

resolve_resource_file core compatibility #73

Open
NeonDaniel opened this issue Jun 3, 2021 · 12 comments
Open

resolve_resource_file core compatibility #73

NeonDaniel opened this issue Jun 3, 2021 · 12 comments
Assignees

Comments

@NeonDaniel
Copy link
Member

With neon, ovos, chatterbox, and mycroft (or holmesV) cores now installable as different packages, resource files have more locations to be bundled. resolve_resource_file currently checks for mycroft defaults.

A few potential solutions come to mind:

  • maintain a list of known packages to check in the path
  • accept some param in resolve_resource_file to specify a root path (or package name to locate in the path)
  • specify that each core extends resolve_resource_file somewhere and import that method (makes modules less universal)
  • add some search logic to find arbitrary res directories in some set path(s)
@NeonDaniel
Copy link
Member Author

New situation to consider here: I modified yes.voc in neon_core, but this is generally not used since it exists in mycroft/holmesV. Do we want to allow for overloading vocab and merge them together? Or spec priority order somehow?

@JarbasAl
Copy link
Member

JarbasAl commented Jun 3, 2021

proposed order, until one is found:

  • fully qualified file / full path
  • xdg path for user defined overrides
  • list of locations passed to the method (optional)
  • look in bundled ovos_utils resources
  • look into ovos_workshop if its installed (meant for ovos skills/templates)
  • detected core res folder (ovos/chatterbox/neon change this value)

when checking folders we can also:

  • look into snd subdir if file is mp3/wav
  • look into text subdir if file is txt/voc
  • look into lang subfolders

@NeonDaniel
Copy link
Member Author

That order makes sense to me, except I would put core/res above ovos packages to prevent the possibility of a dependency breaking core behavior (ovos_utils adds a new res and the core res is no longer matched).. I think a simple log of which absolute path was matched would be immensely helpful for debugging too.

@JarbasAl
Copy link
Member

JarbasAl commented Jun 4, 2021

part of the purpose of ovos utils is to override mycroft-core resource files, the use case you describe really should use the resolve_resource_file method from core, not from ovos_utils

@NeonDaniel
Copy link
Member Author

Hmm, I guess from my perspective, ovos_utils vocab is essentially equivalent to mycroft-core in terms of priority and neon-core is a higher priority than those. I could see the argument for overriding resolve_resource_file in neon-core to work around this, but I also don't particularly like overloading that function name so much since it makes code less clear; it would be better IMO to have all modules (skills, speech, etc) import the same method from ovos_utils.

@JarbasAl
Copy link
Member

JarbasAl commented Jun 4, 2021

in that case you could use the arg for extra paths to define a high priority neon path

without doing that it is considered a "OVOS resource file", if the underlying core finds this unacceptable and there is chance of breakage with a next version, then that core either should not use this method (use its own) or pass the paths as described above

@NeonDaniel
Copy link
Member Author

in that case you could use the arg for extra paths to define a high priority neon path

That sounds fine to me; really, this only comes up in one or two places (MycroftSkill.voc_match specifically is where I have trouble with added vocab and I know the relative path I want to check first there).

@NeonDaniel
Copy link
Member Author

Regarding naming, would it make sense to refactor the method name here while we're making these changes? Main reasoning is that this will often be used in environments with mycroft.util.resolve_resource_file also available and used within packages. Choosing a different name makes it obvious in a terminal/editor, etc. which method is being used..

Minor annoyance though, so this might not be worth the refactor.

@JarbasAl
Copy link
Member

JarbasAl commented Jun 4, 2021

i forgot about resolve_ovos_resource_file

indeed resolve_resource_file is meant to be equivalent to mycroft version, so im unsure whats the best approach now?

@NeonDaniel
Copy link
Member Author

I think depreciating resolve_resource_file in ovos_utils makes sense (log a warning and call whatever new method until the next minor version to avoid breaking things).. As far as a new method name, I don't have any strong preference... Few that come to mind:

locate_resource/locate_resource_file
find_resource
resolve_generic_resource
search_resource

@NeonDaniel
Copy link
Member Author

From OVOS dev chat:

  • add xdg locations to HolmesV -> holmes.conf defines xdg folder -> skills find user resources in those locations
  • allow adding new paths (can be per core) in holmes.conf -> skills find per fork dedicated locations
  • general ovos_utils sprint to support holmes.conf and XDG, it is behind in both these things!

@NeonDaniel
Copy link
Member Author

I think we may be able to close this issue. The solution I've reached in Neon is:

audio_file = resolve_resource_file(snd_file, config=self.config_core)
if not audio_file:
    audio_file = resolve_neon_resource_file(snd_file)
if not audio_file:
    LOG.error(f"Could not resolve {snd_file}")
    return

I think it's up to the skill/module author to scope where to search for resource files (mycroft, ovos, neon, etc)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants