Skip to content

Commit

Permalink
Split get_skill_config into more specific functions. (#53)
Browse files Browse the repository at this point in the history
* Split get_skill_config into more specific functions.

The get_skill_config method previously attempted to handle a number of cases such as when config
doesn't exist, getting a config for specific skill, or getting all skill configurations. This resulted
in the introduction of a bug in PR #50 where the init command attempted to determine if a skill with the
passed in name already existed. The get_skill_config method would make an empty default configuration
if one didn't exist, but seeing a name passed in would then attempt to read from that empty value and
throw an error. This resulted in a new user being unable to initialize a project. To hopefully avoid
other issues in the future the 4 functions previously contained in the get_skill_config method have
been split into their own smaller methods which the commands can make use of as needed.
  • Loading branch information
snow0x2d0 committed Jan 19, 2022
1 parent 7d95a0f commit 114afc5
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 47 deletions.
1 change: 0 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import re

import pytest

from webex_skills.dialogue.rules import SimpleDialogueStateRule
from webex_skills.models.mindmeld import DialogueState

Expand Down
1 change: 0 additions & 1 deletion tests/test_crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import json

import pytest

from webex_skills.crypto import sign_token, verify_signature


Expand Down
1 change: 0 additions & 1 deletion tests/test_dialogue.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pytest

from webex_skills.dialogue.manager import SimpleDialogueManager

pytestmark = pytest.mark.asyncio
Expand Down
35 changes: 19 additions & 16 deletions webex_skills/cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,28 @@

import typer

CONFIG_DIR = Path(typer.get_app_dir('skills-cli', force_posix=True))
CONFIG_FILE = CONFIG_DIR / 'config.json'

def get_skill_config(name=None):
app_dir = Path(typer.get_app_dir('skills-cli', force_posix=True))
config_file = app_dir / 'config.json'

if not app_dir.exists():
typer.echo('Creating default configuration')
app_dir.mkdir(parents=True)
config = {}
else:
config = json.loads(config_file.read_text(encoding='utf-8')) or {}
def get_skill_config(name: str):
"""Return the configuration for a named skill"""
remotes = get_remotes()
return remotes.get(name)

remotes = config.get('remotes', {})
if not name:
return remotes

remote_config = remotes.get(name)
if not remote_config:
def get_remotes():
"""Returns configuration for all named skills"""
if not CONFIG_FILE.exists():
return {}
config = json.loads(CONFIG_FILE.read_text(encoding='utf-8'))
return config.get('remotes', {})


def get_app_dir(name: str):
"""Returns the app directory path for a named skill"""
config = get_skill_config(name)
if not config:
typer.secho(f'No configured remote with the name {name} found', color=typer.colors.RED, err=True)
raise typer.Exit(1)

return remote_config
return config['app_dir']
22 changes: 5 additions & 17 deletions webex_skills/cli/nlp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,42 +3,30 @@

import typer

from .config import get_skill_config
from .config import get_app_dir
from .helpers import create_nlp

app = typer.Typer(help='Commands for working with NLP models')


# take name to find app path, otherwise default to cwd
@app.command()
def build(
name: Optional[str] = typer.Argument(
None,
help="The name of the skill to build.",
),
):
def build(name: Optional[str] = typer.Argument(None, help="The name of the skill to build.")):
"""Build nlp models associated with this skill"""
app_dir = '.'
if name:
config = get_skill_config(name)
app_dir = config['app_dir']
app_dir = get_app_dir(name)

nlp = create_nlp(app_dir)
nlp.build()


@app.command()
def process(
name: Optional[str] = typer.Argument(
None,
help="The name of the skill to send the query to.",
),
):
def process(name: Optional[str] = typer.Argument(None, help="The name of the skill to send the query to.")):
"""Run a query through NLP processing"""
app_dir = '.'
if name:
config = get_skill_config(name)
app_dir = config['app_dir']
app_dir = get_app_dir(name)

nlp = create_nlp(app_dir)
nlp.load()
Expand Down
17 changes: 8 additions & 9 deletions webex_skills/cli/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import typer

from ..crypto import generate_keys
from .config import get_skill_config
from .config import CONFIG_DIR, CONFIG_FILE, get_remotes, get_skill_config
from .helpers import create_nlp

app = typer.Typer(name='project')
Expand All @@ -25,18 +25,20 @@ def init(
resolve_path=True,
),
secret: Optional[str] = typer.Option(
None, help="A secret for encryption. If not provided, one will be" " generated automatically."
None, help="A secret for encryption. If not provided, one will be generated automatically."
),
mindmeld: Optional[bool] = typer.Option(
False,
help="If flag set, a MindMeld app will be created, otherwise " "it defaults to a simple app",
help="If flag set, a MindMeld app will be created, otherwise it defaults to a simple app",
is_flag=True,
),
):
"""Create a new skill project from a template"""

if not CONFIG_DIR.exists():
CONFIG_DIR.mkdir(parents=True)
# Check for an existing skill name and provide an option to overwrite
if get_skill_config(skill_name):
if CONFIG_FILE.exists() and get_skill_config(skill_name):
if not typer.confirm(
f'A skill named {skill_name} already exists in your configuration. Would you like to overwrite it?'
):
Expand Down Expand Up @@ -134,7 +136,7 @@ def _create_project(
env_file_path = output_dir / '.env'
env_file_path.write_text(env_content)

remotes = get_skill_config()
remotes = get_remotes()
remotes[skill_name] = {
'name': skill_name,
'url': "http://localhost:8080/parse",
Expand All @@ -145,8 +147,5 @@ def _create_project(
'app_dir': str(app_dir),
}

config_dir = Path(typer.get_app_dir('skills-cli', force_posix=True))
config_file = config_dir / 'config.json'

config_file.write_text(json.dumps({'remotes': remotes}, indent=2), encoding='utf-8')
CONFIG_FILE.write_text(json.dumps({'remotes': remotes}, indent=2), encoding='utf-8')
return app_dir
4 changes: 2 additions & 2 deletions webex_skills/cli/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import typer

from .config import get_skill_config
from .config import get_remotes

remote = typer.Typer(name='remote', help='Commands for interacting with running skills')

Expand Down Expand Up @@ -81,7 +81,7 @@ def ls(
name: Optional[str] = typer.Option(None, help="The name of a particular skill to display.")
): # pylint:disable=invalid-name
"""List configured remote skills"""
remotes = get_skill_config()
remotes = get_remotes()
if not remotes:
typer.secho('No configured remotes found', color=typer.colors.RED, err=True)
raise typer.Exit(1)
Expand Down

0 comments on commit 114afc5

Please sign in to comment.