Add option to use postgres backend in import and output #28
Conversation
server/dbutil/dbutil.go
Outdated
| // With checkExisting it will fail if the DB does not exist | ||
| func Open(connection string, checkExisting bool) (DB, error) { | ||
| sqliteReg := regexp.MustCompile(`^sqlite://(.+)$`) | ||
| psReg := regexp.MustCompile(`^postgres(ql)?:.+$`) |
There was a problem hiding this comment.
you are checking only for prefix, right?
Would it be easier to use string.HasPrefix?
and for sqlite to remove prefix you can use string.TrimPrefix
just a suggestion, up to you.
There was a problem hiding this comment.
I like to use regexp if there is any excuse :D
I have to agree with you, this may not be most elegant approach. The tool started simple enough that I just went with pure sql. Then postgres had to be added, and the code was already there for sqlite so... |
cli/export/export.go
Outdated
| stored in the assignments table. | ||
| Usage: export <path-to-origin.db> <path-to-destination.db> | ||
| Usage: export DSN DSN |
There was a problem hiding this comment.
Is DSN better then DB_URL ?
Also related - the requirement on the UI level for export is:
- input: url to point to inernal DB
- output: path in the filesystem to sqlight DB
And viasurvesa for import.
I believe it has advantage of simplicity for this app the user, at the same time agility for its developers (choose any DB)
Could you please update it?
There was a problem hiding this comment.
imo DSN is better. It's standard name for it. If I read DB_URL, I'm not sure what format should be used.
For other points 👍
There was a problem hiding this comment.
👍 a good argument for using DSN for DB part also might be posting something like https://en.wikipedia.org/wiki/Data_source_name :)
There was a problem hiding this comment.
there even a wiki page for it! I just saw it across documentation of different drivers & tools.
There was a problem hiding this comment.
@bzz:
input: url to point to inernal DB
output: path in the filesystem to sqlight DB
Please confirm: you want the output argument to accept only sqlite:// strings, or do you want it to ask for a path? i.e.
export postgresql://user:password@host:port sqlite:///path/to/db.dbexport postgresql://user:password@host:port /path/to/db.db
There was a problem hiding this comment.
2 in my opinion follows the right contract.
Import would be import /path/to/db.db postgresql://user:password@host:port
I guess this PR could also take care of doc update, after #29 is merged.
Not yet implemented, but the commands will use filepaths for the input and output DB, limited to SQLite See PR src-d#28: src-d#28 (comment) Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
| "regexp" | ||
| "strings" | ||
|
|
||
| _ "github.com/lib/pq" |
There was a problem hiding this comment.
There was a problem hiding this comment.
makes sense. I'll rebase from master and update dep.
Not yet implemented, but the commands will use filepaths for the input and output DB, limited to SQLite See PR src-d#28: src-d#28 (comment) Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
Arguments can now be sqlite:///path/to/db.db or postgresql://user:password@host:port Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com> Add posgres DB option to the export command Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
Only for the "external" DB, internal can still be sqlite:// or posgresql:// Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
carlosms
force-pushed
the
postgres-cli
branch
from
0a6a765 to
3928c5e
Compare
add tests to src-d/ci itself
4 participants
This commit adds the functionality for the import tool (#4).
A pending commit will add the same options to the export tool (#5)
The basic change is that arguments can now be:
sqlite:///path/to/db.db
postgresql://user:password@host:port