7 Feb
2018
7 Feb
'18
7:13 p.m.
This module implements a generic db driver for kamailio. It
requires a "schema" and "key" definition of "tables" and
corresponding
keys for redis in the kamailio config file, otherwise it's supposed to
work with every module.
Implemented methods are query (w/o order-by), insert, update, delete.
Tested with usrloc and acc.
<!-- Kamailio Pull Request Template -->
<!--
IMPORTANT:
- for detailed contributing guidelines, read:
https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md
- pull requests must be done to master branch, unless they are backports
of fixes from master branch to a stable branch
- backports to stable branches must be done with 'git cherry-pick -x ...'
- code is contributed under BSD for core and main components (tm, sl, auth, tls)
- code is contributed GPLv2 or a compatible license for the other components
- GPL code is contributed with OpenSSL licensing exception
-->
#### Pre-Submission Checklist
<!-- Go over all points below, and after creating the PR, tick all the checkboxes that
apply -->
<!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from
above-->
<!-- If you're unsure about any of these, don't hesitate to ask on sr-dev
mailing list -->
- [x] Commit message has the format required by CONTRIBUTING guide
- [x] Commits are split per component (core, individual modules, libs, utils, ...)
- [x] Each component has a single commit (if not, squash them into one commit)
- [ ] No commits to README files for modules (changes must be done to docbook files
in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change
- [ ] Small bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds new functionality)
- [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist:
<!-- Go over all points below, and after creating the PR, tick the checkboxes that
apply -->
- [ ] PR should be backported to stable branches
- [x] Tested changes locally
- [x] Related to issue #1137
#### Description
<!-- Describe your changes in detail -->
This module implements a generic db driver for redis, to be used by any module (with the
help of some schema configuration in the config file).
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1432
-- Commit Summary --
* db_redis: Implement db_redis generic db driver
-- File Changes --
M src/Makefile.groups (2)
A src/modules/db_redis/Makefile (37)
A src/modules/db_redis/README (199)
A src/modules/db_redis/db_redis_mod.c (109)
A src/modules/db_redis/db_redis_mod.h (31)
A src/modules/db_redis/doc/Makefile (4)
A src/modules/db_redis/doc/db_redis.xml (37)
A src/modules/db_redis/doc/db_redis_admin.xml (190)
A src/modules/db_redis/redis_connection.c (287)
A src/modules/db_redis/redis_connection.h (71)
A src/modules/db_redis/redis_dbase.c (2056)
A src/modules/db_redis/redis_dbase.h (92)
A src/modules/db_redis/redis_table.c (697)
A src/modules/db_redis/redis_table.h (63)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1432.patch
https://github.com/kamailio/kamailio/pull/1432.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432
8 Feb
8 Feb
3:28 p.m.
@agranig pushed 1 commit.
dcb8611 db_redis: Cleanups and fixes
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/1432/files/aab86a5595b4b6294d9a10…
5:10 p.m.
@agranig pushed 1 commit.
2378fb9 db_redis: Fix missed freeing redis reply objects
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/1432/files/dcb86110b481a32071d1b8…
9 Feb
9 Feb
4:35 p.m.
IMO, this one can be merged.
I haven't had the time for an in depth analyzis, spotted a small type in the readme,
ndb_redis can be used to send any command to redis.
Then, as you expressed in some comments, having schema definition in a file may make
possible to generate it from the xml db schema definitions from src/lib/srdb1/schema, in a
simialar matter done for `db_text` or `db_mongodb`. If the parameter is kept, might be
good to use function callbacks for it in order to allow setting it many times without
overwriting previous value, otherwise its value can become too big for a single modparm
(e.g., like done for htable parameter for htable module). Anyhow, these are enhancements
that can be done over the time, as I said, merging at this phase is ok from my point of
view, it will make it easier for others to test and improve its stability.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-364435219
4:36 p.m.
miconda approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#pullrequestreview-95420639
13 Feb
13 Feb
4:59 p.m.
@agranig pushed 1 commit.
609c350 db_redis: Use schema files and improve keys def
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/kamailio/kamailio/pull/1432/files/53e746b5c527fa542e4b2b…
5:01 p.m.
Hi @miconda, I've improved the module with the suggestions you made:
* utilize xml schema definitions and load them from files to avoid having to define them
as mod params
* allow multiple keys mod params definitions to make config more readable
* reconnect improvements
* documentation fixes and improvements
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-365275003
14 Feb
14 Feb
12:28 p.m.
Merged #1432.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#event-1473375435
1:33 p.m.
A quick note that I updated the path for including hiredis.h:
```
diff --git a/src/modules/db_redis/redis_connection.h
b/src/modules/db_redis/redis_connection.h
index d6ed4e4ec3..35d58e46a4 100644
--- a/src/modules/db_redis/redis_connection.h
+++ b/src/modules/db_redis/redis_connection.h
@@ -23,7 +23,7 @@
#ifndef _REDIS_CONNECTION_H_
#define _REDIS_CONNECTION_H_
-#include <hiredis.h>
+#include <hiredis/hiredis.h>
#include "db_redis_mod.h"
```
On Debian 9 is where the file is located based on pkg-config output, it doesn't
compile otherwise (same on MacOS with macports).
I looked in ndb_redis module and has the same path. If you have it in your system working
without `<hiredis/hiredis.h>`, then we need to do some defines to set the proper
path so it works on common distros.
Thanks again for this contribution!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-365562086
21 Feb
21 Feb
12:33 p.m.
The relevant question - how to implement authentication while connecting to redis
storage?
I read this document: https://kamailio.org/docs/modules/devel/modules/db_redis.html
and didn't find any reference in it.
As I understood we use following syntax to connect to the redis db:
redis://[username]@host:port/database
But authentication capability is not provided as I can see.
Of course there is an excellent module named ndb_redis.
But my main goal is to perform authentication with cnxcc module, which is not able to do
that (Carlos Ruiz Díaz will add this possibility later?).
Please correct me if I'm wrong.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-367265258
9:11 p.m.
Try redis://password@host:port/db and let me know if it works for you.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-367418291
23 Feb
23 Feb
6:31 p.m.
Well, for that moment it doesn't work.
My configuration is as follows:
#!define DBURL_CNXCC "redis://here_my_password@redis.domain.com:6379/1"
loadmodule "db_redis.so"
modparam("db_redis", "schema_path",
"/usr/share/kamailio/db_redis/kamailio")
modparam("cnxcc", "redis", DBURL_CNXCC)
I think this is wrong way to do that.
cnxcc module uses redis db as htable and stores real-time values (like credit, call
duration etc.) that exist only when call is up for certain customer, then it drops whole
key.
Should I use: 'modparam("db_redis", "keys"...' ?
Or db_redis is to be improved to work with cnxcc? (cuz it uses not "db_url"
modparam but "redis" modparam).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-368041757
6:40 p.m.
@zenichev - cnxcc connects directly to redis, doesn't use a DB connector at all, such
as db_mysql or db_redis. In other words, there is no relation between db_redis and cnxcc.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-368044418
6:44 p.m.
@miconda - as I mentioned above, I need working authentication for cnxcc (that is not able
to work with authentication).
So that, I'm trying to find a way to do that, this time by db_redis.
Do you think this is a wrong way?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-368045881
6:48 p.m.
@zenichev - as said, db_redis cannot be used with cnxcc. Start a discussion on
sr-users(a)lists.kamailio.org with what you need to achieve. The issue tracker is only for
bug reports, pull requests should be discussed in the context of the committed code: to
review, improve, etc... it is not how to use the new features.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-368047152
6:53 p.m.
@miconda Ok I understood it clearly. Thank you for your answer.
Will try to start discussion on sr-users list.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1432#issuecomment-368049038
2408
days inactive
2424
days old
15 comments
3 participants
participants (3)
-
Andreas Granig -
Daniel-Constantin Mierla
-
zenichev