sukeesh / Jarvis

Personal Assistant for Linux and macOS
MIT License
3.03k stars 1.04k forks source link

Add database to Jarvis #524

Closed JonanOribe closed 5 years ago

JonanOribe commented 5 years ago

Maybe is time to add a database to Jarvis because if we want that it really works as personal assistant, it will need to know the user it works for. You know...what stuff the user likes, user's personal configuration, agenda,etc..So, what do you think?

pnhofmann commented 5 years ago

Actually - there is already:

https://github.com/sukeesh/Jarvis/tree/master/jarviscli/packages/memory

Also take a look at:

https://github.com/sukeesh/Jarvis/blob/master/doc/API.md#get_data

Basically just writes data to a json file - so really simple and can certainly be improved. But is already used for example to save users location.

shreyashag commented 5 years ago

I am willing to take this up. Redis implementation seems to be simple enough.

If we are going to add a connection, we need to store the connection URL somewhere, like a config file. Is this community opinionated towards some kind of markup? (INI/YAML/XML/etc.)

pnhofmann commented 5 years ago

Thanks, unfortunately Redis (server) is written in C and therefore needs compilation + installation not possible with pip only.

Non-python dependencies is something we avoid - especially for core components.

Actually I don't really see why our current approach (store data in json file + use simple python dictionary) shouldn't be enough for what we do. But if we really need a real database, I would prefere something like sqlite.

appi147 commented 5 years ago

I second @pnhofmann . We really don't need a database. JSON is quite simple and fast. Also, all the methods are quite implemented and documented. If there is something you want to improve in this system, it is welcomed

shreyashag commented 5 years ago

If there is provision for storing credentials, existing functionality can be extended.

Example, email credentials can be stored, and additional functionalities can be added, such as storing the login.

Willing to implement the same with sqlite also.

shreyashag commented 5 years ago

534

Have added a fix for memory as I would like to contribute to this area of Jarvis!

appi147 commented 5 years ago

If there is provision for storing credentials, existing functionality can be extended

Storing of credentials is not a good idea. We can accept PR if you want to contribute. But implementation should be simple and fast

And, if possible, using python

shreyashag commented 5 years ago

Why is storing credentials not a good idea?

An assistant needs to know who you are. It carries an overhead of setting/clearing/resetting the config, but it's doable and will allow a lot of customisation with respect to the user.

appi147 commented 5 years ago

If password is to be stored, it doesn't matter if you store in plaintext or database.

appi147 commented 5 years ago

It is too risky.

shreyashag commented 5 years ago

No, don't store passwords. Store auth credentials that can be used for API access, for example google Oauth to access Gmail API and contacts.

appi147 commented 5 years ago

@pnhofmann

pnhofmann commented 5 years ago

No problem with auth credentials - but don't see a difference if it's in a database or plain json file. I have no general problem with database. But a database is just more complicated as current simple json approach and that would need some justification, because a database is not something you integrate just for fun.

So the key question I would ask: @shreyashag What would you do with a database in Jarvis, that our current json file can't do or can't do as good as a database.

shreyashag commented 5 years ago

@pnhofmann In my opinion, having a database seems like a more standard and flexible, albeit cumbersome approach. Here are a few things we could do-

  1. Store user profile information.
  2. Setup daily news crawler in background processes that store news relevant to the user.

On the same note, I realize this discussion is ahead of time. I will think of more concrete and useful examples.

You can mark this as closed if you want.

In the meanwhile, would love to contribute to other areas as well.

I have some experience in building this kind of a system where I used OpenCV to integrate facial detection and recognition among other things. Also integrated kitt.ai ( snowboy decoder for local hotword recognition)

JonanOribe commented 5 years ago

I was thinking in something like @shreyashag said. For example, maybe I want that Jarvis save information about the weather,my agenda, wallstreet, whatever, because at the end of the week I want a summary or draw a chart with the evolution of the prices of "x" value.I see the database as a fundamental for long-term processes, so then we could ask for "time series" instead of about today's value of something.

appi147 commented 5 years ago

It can be added. If anyone in future wants to use it in their future, why stop them right? We just could not think of scenario, where our existing system can't be used. Its simple to use, its fast, and user can manually enter their or update their values manually in any editor.

pnhofmann commented 5 years ago

Agree with appi147 and I'm closing this issue - if someone needs a database, feel free to implement, but we won't add a database just in case it could be needed.