Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

feat/sql_alchemy #51

Merged
merged 24 commits into from
Apr 7, 2023
Merged

feat/sql_alchemy #51

merged 24 commits into from
Apr 7, 2023

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Mar 17, 2023

support multiple SQL engines

Json database was good for prototyping but does not scale

adopts the voice_config concept also defined here OpenVoiceOS/ovos-plugin-manager#131

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

❗ No coverage uploaded for pull request base (dev@8aa8288). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 25ce9a2 differs from pull request most recent head 25be77e. Consider uploading reports for the commit 25be77e to get more accurate results

@@          Coverage Diff          @@
##             dev     #51   +/-   ##
=====================================
  Coverage       ?   0.00%           
=====================================
  Files          ?      20           
  Lines          ?    1664           
  Branches       ?       0           
=====================================
  Hits           ?       0           
  Misses         ?    1664           
  Partials       ?       0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@emphasize

This comment was marked as resolved.

support multiple SQL engines

Json database was good for prototyping but does not scale

_________

Fix/table population (#52)

_____________

16MB per sample

location data

db from config

mysql compat

get_device

get_device

add/update_device

Device.deserialize

Device.serialize

properties

save_ww_recording

save_stt_recording

save_metric
@JarbasAl JarbasAl added enhancement New feature or request breaking labels Mar 30, 2023
@JarbasAl JarbasAl marked this pull request as ready for review March 30, 2023 10:26
Copy link
Member

@builderjer builderjer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything that I currently use in a backend seems to be working fine. Pairing new devices, weather api key, time format, location, geolocation, all work. I don't currently use oauth, so no real way to test it. I am just storing stuff in the default sqlite db. Have not tried to connect to my personal mysql db.

No errors that I have seen in the logs

@JarbasAl JarbasAl requested a review from emphasize March 31, 2023 15:14
@JarbasAl JarbasAl requested a review from builderjer March 31, 2023 16:33
@emphasize
Copy link
Member

emphasize commented Apr 2, 2023

opt_in is sent as False, although enabled in CONFIGURATION

ok, got cha, this is only handled in the table now (brought my own old config)

_mail_cfg.get("recipient") or \
_mail_cfg.get("smtp", {}).get("username")

loc = location or _loc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line does not work. location is not an empty dict so it never gets to _loc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

location['coordinate'] is the only one that throws an error, so I replaced with a check for that.

if not location["coordinate"]:
    loc = _loc
else:
    loc = location

then it worked. Not sure how else to handle this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not following, where is that coming from? we certainly dont want to partially read the default loc only, its all or nothing

is something passing an incomplete location here?

@JarbasAl JarbasAl merged commit 48f763f into dev Apr 7, 2023
@JarbasAl JarbasAl deleted the feat/sql_alchemy branch April 7, 2023 01:03
requirements/requirements.txt Show resolved Hide resolved
ovos_local_backend/backend/device.py Show resolved Hide resolved
ovos_local_backend/backend/crud.py Show resolved Hide resolved
ovos_local_backend/database.py Show resolved Hide resolved
ovos_local_backend/backend/precise.py Show resolved Hide resolved

add_ww_recording(uuid,
audio,
meta.get("name", "").replace("_", " "),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to the backend-client
we have to access wake_word (or change it there)
A reason why i changed the variables, this is done numerous times back and forth. A debugging hell.

ovos_local_backend/database.py Show resolved Hide resolved
@emphasize emphasize mentioned this pull request Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants