Skip to content
This repository has been archived by the owner on Jan 9, 2019. It is now read-only.

Added sparql_insert action #11

Merged
merged 3 commits into from Apr 3, 2014
Merged

Added sparql_insert action #11

merged 3 commits into from Apr 3, 2014

Conversation

fasseg
Copy link
Contributor

@fasseg fasseg commented Mar 6, 2014

This adds an action to bechtool allowing the benchmarking of a simple sparql insert query

@Override
protected long sparqlInsert(String pid, TransactionState tx)
throws IOException {
String uri = this.fedoraUri + "/rest/objects/" + pid;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inserting sparql should optionally work with transactions, so just need to swap out the this.fedoraUri bit with getFedoraRestUri(tx)

@bbpennel
Copy link
Contributor

bbpennel commented Mar 6, 2014

The ticket mentions "SPARQL_UPDATE and SPARQL READ", it looks like this PR just includes SPARQL_INSERT though. Do you want to include SPARQL_READ in this PR or in a different one?

Also, minor note, but I think your IDE is configured to 80 character line lengths, which caused a lot of unrelated whitespace changes in BenchTool.java

You will probably also need to pull from master since this PR is behind the no-purge-switch commit now

@Override
protected long sparqlSelect(String pid, TransactionState tx)
throws IOException {
String sparqlUri =this.fedoraUri + "/rest/fcr:sparql";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sparql select does not work with transactions it seems since the fcr:sparql endpoint does not recognize the tx path part of the URI

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kind of surprised since it works with sparql insert. It could just be because of this other ticket https://www.pivotaltracker.com/story/show/66262856 ?

@fasseg
Copy link
Contributor Author

fasseg commented Mar 7, 2014

@bbpennel I think we agreed on 80 characters width for the eclipse project. At least that's what I have in my fedora-specific eclipse settings...

@ajs6f
Copy link

ajs6f commented Mar 7, 2014

@fasseg : No, we switched that to 120 a few months ago. Check with @awoods for the updated settings.

@bbpennel
Copy link
Contributor

bbpennel commented Mar 7, 2014

This just needs to be rebased to master than its good to go

fasseg pushed a commit to fcrepo/fcrepo that referenced this pull request Apr 3, 2014
@fasseg
Copy link
Contributor Author

fasseg commented Apr 3, 2014

I rebased on master an changed the formatting in a separate commit.

bbpennel added a commit that referenced this pull request Apr 3, 2014
@bbpennel bbpennel merged commit d4980db into master Apr 3, 2014
@bbpennel
Copy link
Contributor

bbpennel commented Apr 3, 2014

It all looks good now, I will probably update my transaction bench marking statistics in the wiki to include sparql_insert soon.

@awoods awoods deleted the sparql-update branch April 4, 2014 00:34
@awoods
Copy link

awoods commented Apr 4, 2014

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants