-
Notifications
You must be signed in to change notification settings - Fork 64
user database #24
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
user database #24
Conversation
|
Further considerations, after looking at https://github.com/NeonGeckoCom/neon-core/blob/b0e2bafcbdc8de472bfb5ffa9dd752e23149859e/NGI/utilities/chat_user_util.py TODO
typical data dict user.data = {
"first_name": "Jon",
"middle_name": "Something",
"last_name": "Doe",
"nicknames": ["Dude", "FuriousBadger"],
"birthday": "YYYY/MM/DD",
"about": "very nice user, likes cookies",
"phone": "666",
"avatar": "human.jpg"
}typical preferences dict user.preferences = {
"name": "Master",
"units": "metric",
"date_format": "DD/MM/YYYY",
"time_format": 24,
"tts": {"gender": "female", "voice": "Joanna"},
"stt": {"lang": "en", "region": "US"},
"brands": {"favorite": [], "ignored": []},
"synonyms": {"a": "means b"}
}typical auth dict user.auth = {
"phone_verified": False,
"email_verified": True,
"key": "SecretUsedForAuthentication",
"secret_phrase": "Tell me your secret phrase vocal interaction / password recovery",
"HiveMind": "api_key"
} |
|
@NeonDaniel question, what kinda of things do you usually need to query a database for? users by email, name, api_keys etc.. i'm asking so i can add additional utility methods (that work with any database backend) and optimize the sqlalchemy model (to allow SQL queries and filtering, not available for fields inside data/preferences/auth by default) |
|
You should look at our user yml to see what kinds of data we keep per-user and how we have it organized (also note the device section at the bottom is per-device, not user and should probably move to local_conf now that I look at this again). We query sql using our 'nick' param (unique user ID) and then build a dictionary of user profile settings that match up with what we have in our yml user, brands, speech, location, and unit sections. api keys and stt/tts module settings are not user-specific; we included them in the user yml with the intention of having configuration being portable, so a user could use a common config on different physical devices (later maybe handle synchronization via klat account). We reduced SQL queries by saving profiles in listener with socketIO emits to notify when a reload from SQL for a given user is necessary. The ChatUser object model is a leftover from a previous method of getting user profile data, so we don't really need to initialize the object anymore, we could make the utility just return the dictionary directly. |
44bd83f to
4db3974
Compare
|
added geolocation utils, a Location object, result caching, user properties described above I think it makes sense to drop nick as the unique id, i will still add support for querying by nickname, but i'm designing this in a way that also works 100% by voice on a local device, where a nick does not make sense. the unique identifier now is numeric |
|
also note, the standard way to access an user is now by message.user This means user data can come from anywhere in the pipeline, and is available anywhere in the pipeline. It is important to use message.reply / message.forward everywhere to ensure user data is passed along every time. Source of utterances should always include user data (server/speech_client). server messages are processed in #26, the user object is constructed from the message context Follow up PR adding a voice equivalent to #18 will recognize users from speech in the listener section, this may also apply to server utterances where the data is assumed to be present already, in which case a voice_id confidence field may be added. |
| # "tts": {"gender": "female", "voice": "Joanna"}, | ||
| # "stt": {"lang": "en", "region": "US"}, | ||
| # "brands": {"favorite": [], "ignored": []}, | ||
| # "synonyms": {"a": "means b"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: synonyms handling will be a new module for #18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually looked at this and I think synonyms would make more sense as a skill preference, since they are dependent on that skill and not really anything else; the skill can just handle the utterance in converse and would then be Mycroft-compatible too
| # "date_format": "DD/MM/YYYY", | ||
| # "time_format": 24, | ||
| # "tts": {"gender": "female", "voice": "Joanna"}, | ||
| # "stt": {"lang": "en", "region": "US"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tts + stt prefered language should be set here and consumed in appropriate places
| # "first_name": "Jon", | ||
| # "middle_name": "Something", | ||
| # "last_name": "Doe", | ||
| # "nicknames": ["Dude", "FuriousBadger"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fields above will be used in match_user for fuzzy matching, in general only field from user.data will be used for matching
user_id match will take precedence, user.auth wil also take precedence
| data = json.loads(data) | ||
| self.latitude = data["coordinate"]["latitude"] | ||
| self.longitude = data["coordinate"]["longitude"] | ||
| self._city = data["city"]["name"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all coordinates have a city name; how is this handled in your geolocate package? In Nominatim, I used 'county' as a more-specific location when city was unavailable
|
|
||
| @property | ||
| def blacklisted_skills(self): | ||
| return self.auth.get("blacklisted_skills", []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the current/Mycroft implementation here, is this really possible per-user, or is this per-core (or both?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right this is not possible, it will require hivemind integration, i will remove it for now
|
How would a user manage their profile on a local install? With My preference would be to use a yml config for persistence on local installs and the existing SQL database for the sever as we do now. This would mean the UserDatabase acts as a cache to avoid reading from disk/SQL query when values are unchanged. My main concern is exposing some means for easily updating profile fields that aren't directly accessible via intent. |
|
I think this functionality might be best implemented as a separate (optional) module. For server implementations, this data is expected to be present in the message before it gets to the skills module; for a local implementation, maybe passing messages through a module just before the intent service (before text parsers?) would be the easiest implementation. |
Initial implementation for a user database