Merge lp:~3v1n0/u1db-qt/uri-path-parsing into lp:u1db-qt
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Christian Dywan on 2015-03-05 | ||||
Approved revision: | 126 | ||||
Merged at revision: | 128 | ||||
Proposed branch: | lp:~3v1n0/u1db-qt/uri-path-parsing | ||||
Merge into: | lp:u1db-qt | ||||
Diff against target: |
174 lines (+70/-17) 4 files modified
examples/u1db-qt-example-1/u1db-qt-example-1.qml (+1/-1) src/database.cpp (+35/-13) src/database.h (+4/-1) tests/test-database.cpp (+30/-2) |
||||
To merge this branch: | bzr merge lp:~3v1n0/u1db-qt/uri-path-parsing | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot | continuous-integration | Approve on 2015-03-05 | |
Christian Dywan | 2015-02-28 | Approve on 2015-03-05 | |
Review via email:
|
Commit message
Database: support parsing of URI paths using something such as "file://"+ path
Description of the change
Make the database to parse and use local URIs starting with file://
Unfortunately this does not fix lp:1426178, which seems related to something
else.
Christian Dywan (kalikiana) wrote : | # |
It seems unexpected to me that passing a URI would set the property to a different value - an API consumer should be able to assume that you can get back what you set. So initializeIfNeeded should be where the URI is supported, which is also where :memory: is special-cased. And maybe in the unit test we can check that there's no error in addition to comparing the path?
- 125. By Marco Trevisan (Treviño) on 2015-03-05
-
Database: don't change the internal m_path value when sanitizing, just keep the user value
And sanitize it when creating the Db
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:127
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Christian Dywan (kalikiana) wrote : | # |
You left out the bit where the folder is being created. That is needed.
- 126. By Marco Trevisan (Treviño) on 2015-03-05
-
TestDatabase: open temporary files, to actually create paths
And update QUrl test
Christian Dywan (kalikiana) wrote : | # |
I didn't realize how the change of the if/else in fact deduplicate code here, so in fact it's all good now and a nice clean-up on top of it! Thanks a lot!
- 127. By Marco Trevisan (Treviño) on 2015-03-05
-
Database: keep the ":memory:" special path saved in Database:
:MEMORY_ PATH - 128. By Marco Trevisan (Treviño) on 2015-03-05
-
Database: make sure that setting an empty path, moves the db to memory
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Cool... On a side: is there any reason why the path is not set as ":memory:" by default (but as an empty string)?
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:126
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Christian Dywan (kalikiana) wrote : | # |
The original reasoning is this: If no path is set the database is in memory. Unlike the Python implementation the code path is exactly the same as using a file, relying on sqlite's support for :memory:. Only the explicit support for relative paths and now URIs requires code that is aware of the :memory: case. API users don't have to know about it either.
PASSED: Continuous integration, rev:124 jenkins. qa.ubuntu. com/job/ u1db-qt- ci/61/ jenkins. qa.ubuntu. com/job/ u1db-qt- vivid-amd64- ci/8 jenkins. qa.ubuntu. com/job/ u1db-qt- vivid-armhf- ci/8
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/u1db- qt-ci/61/ rebuild
http://