skills-online-mysql missing locally & Local installation steps in README are outdated

[Auenc]

When trying to run the project locally, the Makefile references a docker image that isn't created via the build-local command:

run-local:
	docker run --name techiaith-padatious -it --rm \
		--link skills-online-mysql:mysql \
		-v ${PWD}/server/assistant/:/opt/padatious/src \
		-v ${PWD}/recordings/:/recordings \
		-v ${PWD}/data/:/data \
		-v ${PWD}/local/:/opt/padatious/local \
		techiaith/padatious bash

Additionally, the installation steps in the README seem to be outdated as the reference Make targets that do not exist in the current form of the make file:

make build-online-api
make run-online-api

Apologies if I have somehow lost the steps to rectify these issues via attempting to translate the README into English!

[DewiBrynJones]

Helo,

I'm sorry about the documentation. It isn't that well documented in English or Welsh.

When trying to run the project locally, the Makefile references a docker image that isn't created via the build-local command

Are you sure? make build-local builds ..

REPOSITORY              TAG                     IMAGE ID       CREATED         SIZE
techiaith/padatious     latest                  265f16d3d830   8 minutes ago   717MB

can you see this under docker images?

After running make build-localor make build, you need to run make mysql to make sure make run has a database to link to.. (https://github.com/techiaith/macsen-sgwrsfot/blob/master/Makefile#L9)

Then you can type in the docker container:

root@6624ca6f0420:/opt/padatious# python3 src/Brain.py --text "Beth yw'r newyddion?" 

but it will give you an error..

Traceback (most recent call last):
  File "src/Brain.py", line 102, in <module>
    brain = Brain(online=False)
  File "src/Brain.py", line 20, in __init__                                                                                                                     self.load_skill(skills_root_dir, 'tywydd')                                                                                                                File "src/Brain.py", line 41, in load_skill                                                                                                                   skill_python_module = importlib.import_module('skills.%s.%s' % (skillname, skillname))                                                                    File "/usr/lib/python3.5/importlib/__init__.py", line 126, in import_module                                                                                   return _bootstrap._gcd_import(name[level:], package, level)                                                                                               File "<frozen importlib._bootstrap>", line 986, in _gcd_import                                                                                              File "<frozen importlib._bootstrap>", line 969, in _find_and_load
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 673, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 665, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/opt/padatious/src/skills/tywydd/tywydd.py", line 13, in <module>
    from .owm.apikey import OWM_API_KEY
ImportError: No module named 'skills.tywydd.owm.apikey'

unless you've followed the README here..

https://github.com/techiaith/macsen-sgwrsfot#llwytho-i-lawr-a-gosod-y-sgwrsfot

and installed your own API keys into the required skills.

But I agree and will resolve that the README needs updating and translated into English as well.

Diolch yn fawr.

[DewiBrynJones]

see a82d7f7

contains a small improvement to the README. But no translation into English yet. Feel free to suggest or hightlight any other deficiencies that frustrates running macsen-sgwrsfot locally.

Diolch!

[Auenc]

Helo!

Sorry for the late reply, been a little busy today.

Managed to get the local docker setup working with your instructions, thank you!

One thing I'm strugling with at the moment, and I think it would be good to update the README to include this, but what exactly should the API keys be named? Perhaps it would be good to show an example of the apiKey.py file with empty strings. If we can do this, then it would probably be good to split the API key steps into its own section; such as "configuration". That way if someone is returning to the README to remind themselves how to run the program locally, they don't have to go through the API key setup.

Additionally I think we should remove the $ from the example commands. That way people can copy & paste the commands , and assuming they have setup the API keys, have the program running without having to manually type anything.

I'll make a PR showing what I mean once I know the names of the API key variables.

I also wonder if the service should check if the API keys exist on start up, and shutdown if they are not.

[DewiBrynJones]

Helo

The variables are named in the relevant skills here:

https://github.com/techiaith/macsen-sgwrsfot/blob/master/server/assistant/skills/tywydd/tywydd.py#L13

https://github.com/techiaith/macsen-sgwrsfot/blob/master/server/assistant/skills/amser/amser.py#L9

https://github.com/techiaith/macsen-sgwrsfot/blob/master/server/assistant/skills/spotify/api/search.py#L4

You could I guess leave these empty, so that the server doesn't crash at startup and as long as you don't execute the skill. (i.e. you're testing Newyddion only, which doesn't have an third party api key)

We've ommited all the enclosing apikey.py files so that we can easily prevent inadvertantly pushing our api keys to a public repo.

Diolch!!

p.s.

Sorry for the late reply, been a little busy today.

No worries. I'm on holiday at the moment, so take your time ;)

[Auenc]

Diolch yn fawr!

You could I guess leave these empty, so that the server doesn't crash at startup and as long as you don't execute the skill. (i.e. you're testing Newyddion only, which doesn't have an third party api key)

That makes sense.

We've ommited all the enclosing apikey.py files so that we can easily prevent inadvertantly pushing our api keys to a public repo.

I agree with this, I was more suggesting that in the README we show an example of how the apiKey.py should look. Such as something like this:

OWM_API_KEY = "" # Open Weather API Key
TIMEZONEDB_API_KEY = "" # TimeZoneDB API Key
CLIENT_ID = "" # Spotify ClientID
CLIENT_SECRET = "" # Spotify ClientSecret

Still not providing the file within the repo, and keeping the gitignore but allowing a new user to have a skeleton to build their keys off of. Alternatively, I have also seen people commit a skeleton such as above in an example file such as apiKey.example.py. Though that still allows for the potential of people accidentally adding their keys to the example file and commiting the changes.

Ah in that case, enjoy your holiday!

Diolch eto!