New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Stardog as Backend Adapter #122
Conversation
Guzzle installation seems to fail in Travis, can you please look into that @simeonackermann ? |
Guzzle requires PHP >= 5.5.0, but Travis was used with 5.4. Could this be updated? |
Maybe we use Guzzle in version 5.3.1 for now? It supports PHP 5.4. |
Unfortunately its not compatible with current implementation, I have to change some methods first... |
Could you please recheck Travis? |
Travis didn't build this pull request, maybe you have to resolve the merge conflict first. For instance by re-basing on the current develop. |
Ok, thanks for the hint, Travis build with success :) |
@simeonackermann: Please fix the conflicts (again) so that we can merge your code. |
Can you please rebase your commits on the current develop branch instead of merging it in? At least we want to get rid of the "Merge..." commits, but maybe you can also rework your commits into logical chunks? Use |
5686b7f
to
c459882
Compare
@@ -26,7 +26,8 @@ | |||
"pdepend/pdepend": "2.2.*", | |||
"phploc/phploc": "2.1.*", | |||
"phpmd/phpmd": "2.4.*", | |||
"white-gecko/arrays": "*" | |||
"white-gecko/arrays": "*", | |||
"guzzlehttp/guzzle": "5.3.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix indentation, use spaces instead of tabs
Please take the following actions: Required for merge
Optional
|
@simeonackermann: Please fix mentioned points of @pfrischmuth before start researching the elevator task. |
@pfrischmuth : The integration test is mainly taken from virtuoso integration test and php codesniffer marks too long lines as:
A workaround might be:
But I think this is not really better to read... |
Maybe not better, but now its align with the coding standard. And if you want to read it, you can do that on a fixed screen area (max. 120 chars in the width?). In the older version you need to scroll far to the right, which is very annoying. |
c459882
to
4d90a62
Compare
Final required actions before merge
|
Hm, my integration tests completes OK, with: Tests: 68, Assertions: 219, Skipped: 16 and no errors for me... |
Could you please post the test error log? |
When I run the tests, the output is not deterministic... the number of errors/failures differ from time to time. I run stardog and mysql in a docker and run the test with docker also. The commnad is: The output is:
|
4d90a62
to
3105977
Compare
|
The Psr7 method is from previous guzzle version an needs to removed. But even without I get an error like:
|
3105977
to
ebe643c
Compare
Last commit fixes undefined function, but still having some problems with OntoWiki. |
With your latest fix I can run OntoWiki with Stardog backend without errors. When I try to execute a SPARQL query |
- testet with Stardog >=4.0.1 (available via Docker) - use Zend_Http_Client as http request client - add sample config.ini - add integration tests - currently no unit tests - currently no native rdf import/export - code formatted with php code sniffer
ebe643c
to
5aefd29
Compare
Beside the wrong Sparql results I still had the issue with the destroyed Zend session (tried a lot to fix it but doesnt found the problem). I only figured out that something with Guzzle caused my issue, so I replaced Guzzle with the |
if ( ! isset($options['base_url']) ) { | ||
throw new Erfurt_Store_Adapter_Exception('Your config.ini lacks a store.stardog.base_url parameter.'); | ||
} | ||
$this->_baseUrl .= ( substr($options['base_url'], -1) == "/" ) ? $options['base_url'] : $options['base_url'] . "/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix to long line
5aefd29
to
3799657
Compare
|
LGTM |
Thanks guys! 👍 |
This implements Stardog as new backend adapter, originally inspired by @Matthimatiker in #86
It uses Guzzle 5.3 as HTTP client to request Stardog via HTTP Interface.
All operations as used for Virtuoso to query the store, add or remove statements should be supported. If versioning is enabled (default) an additional SQL ZendDB store is required.
The implemented integration test (
make test-integration-stardog
) completes with success.Stardog can be installed as Docker Container from https://github.com/Dockerizing/Stardog
config.ini
for Erfurt with Stardog should look like: