You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2020/11/03 14:17:34 UTC
[GitHub] [jena] rvesse opened a new pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
rvesse opened a new pull request #822:
URL: https://github.com/apache/jena/pull/822
Some of the implementation for adding a `/$/compact/*` endpoint to a Fuseki server that allows live database compaction via the REST API
To Do:
- [ ] Add some integration test cases
- [ ] Companion PR for doc updates
- [ ] Enable this and other relevant endpoints via the command line deployment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #822:
URL: https://github.com/apache/jena/pull/822#issuecomment-727637195
(without advocacy)
@rvesse - do you want to get any of this into 3.17.0?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r516813995
##########
File path: jena-fuseki2/jena-fuseki-webapp/pom.xml
##########
@@ -56,6 +56,12 @@
<artifactId>jena-text</artifactId>
<version>3.17.0-SNAPSHOT</version>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.jena</groupId>
+ <artifactId>jena-tdb2</artifactId>
+ <version>3.17.0-SNAPSHOT</version>
+ </dependency>
Review comment:
core has a dependency on jena-tdb2 via apache-jena-libs.
I also check by adding a call to `DatabaseMgr.compact` in a class jena-fuseki-webapp.
`(cd jena-fuseki-webapp ; mvn dependency:tree)` on the default branch of the codebase gives me:
```
[INFO] org.apache.jena:jena-fuseki-webapp:jar:3.17.0-SNAPSHOT
[INFO] +- org.apache.jena:jena-fuseki-core:jar:3.17.0-SNAPSHOT:compile
[INFO] | +- org.apache.jena:apache-jena-libs:pom:3.17.0-SNAPSHOT:compile
[INFO] | | +- org.apache.jena:jena-shacl:jar:3.17.0-SNAPSHOT:compile
[INFO] | | | \- org.apache.jena:jena-arq:jar:3.17.0-SNAPSHOT:compile
[INFO] | | +- org.apache.jena:jena-tdb:jar:3.17.0-SNAPSHOT:compile
[INFO] | | +- org.apache.jena:jena-tdb2:jar:3.17.0-SNAPSHOT:compile
```
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
Maybe test it is the right species of DatrasetGraph and return an error if it is not.
See `TDBOps` in fuseki-core.
##########
File path: jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java
##########
@@ -1021,6 +1021,7 @@ private void servletsAndFilters(ServletContextHandler context) {
addServlet(context, "/$/stats/*", new ActionStats());
if ( withMetrics )
addServlet(context, "/$/metrics", new ActionMetrics());
+ // TODO Should we support registering other functionality e.g. /$/backup/* and /$/compact/*
Review comment:
There are a couple of big issues here.
1. Security. So far fuseki-main additional operations under `/$/`are "mostly harmless" but this is a step more impactful.
1. The general area of admin for Fuseki/main where my concern is that we are building in a URL pattern and _maybe_ a better design is add it as a service on the `/myTDB2dataset/compact`.
Suggestion: add this admin function now for fuseki-webapp and fuseki-main later.
Let's discuss on the JIRA ticket as it is not about the jena-webapps code which looks fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r525070328
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
Figured this out with the latest commit, various parts of the async task code was just logging an error and continuing on rather than throwing it upwards which prevented detecting the error at the point where we could set a success flag appropriately. Unit tests are updated to check for this flag as appropriate
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r518667724
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
If the test is in "validate", it is in the request cycle.
The existing, long-time contract of `ActionAsyncTask` and `run()` could do with some clearing up (separate to this PR!). Looks a bit messy / quick hackish. e.g. The `TaskBase` constructor have args for the element of the `HttpAction` rather than the implementation comment + single `HttpAction` arg.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r525055065
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/Backup.java
##########
@@ -84,6 +84,10 @@ public static void backup(Transactional transactional, DatasetGraph dsg, String
* @see #backup(Transactional, DatasetGraph, String)
*/
private static void backup(DatasetGraph dsg, String backupfile) {
+ if (dsg == null) {
Review comment:
Stumbled on this while writing some new unit tests around both `/$/backup/<dataset>` and `/$/compact/<dataset>`
This function didn't verify that `dsg` was not `null` so when it tried to interact with the `ConcurrentHashMap` that stores active backups it would NPE because concurrent maps do not permit `null` keys
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -187,6 +189,21 @@
addTestDataset(fileBase+"config-ds-plain-2.ttl");
checkExists("test-ds2");
}
+
+ @Test public void add_delete_dataset_6() {
Review comment:
Added a TDB2 based dataset so I could test `/$/compact/<dataset>`, since adding and deleting this dataset might be unstable on Windows I made this and other tests that use that dataset require a non-Windows OS
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/JsonConstCtl.java
##########
@@ -25,5 +25,6 @@
public static final String task = "task";
public static final String finished = "finished";
public static final String started = "started";
+ public static final String success = "success";
Review comment:
As noted elsewhere was trying to get a `success` flag in the Task output working but couldn't get this working, left here for future reference
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r518667724
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
If the test is in "validate", it is in the request cycle.
The contract of `ActionAsyncTask` and `run()` could do with some clearing up (separate to this PR!). Looks a bit messy / quick hackish. e.g. The `TaskBase` constructor have args for the element of the `HttpAction` rather than the implementation comment + single `HttpAction` arg.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r516030387
##########
File path: jena-fuseki2/jena-fuseki-webapp/pom.xml
##########
@@ -56,6 +56,12 @@
<artifactId>jena-text</artifactId>
<version>3.17.0-SNAPSHOT</version>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.jena</groupId>
+ <artifactId>jena-tdb2</artifactId>
+ <version>3.17.0-SNAPSHOT</version>
+ </dependency>
Review comment:
This addition was necessary to be able to call `DatabaseMgr.compact()`
##########
File path: jena-fuseki2/jena-fuseki-main/src/main/java/org/apache/jena/fuseki/main/FusekiServer.java
##########
@@ -1021,6 +1021,7 @@ private void servletsAndFilters(ServletContextHandler context) {
addServlet(context, "/$/stats/*", new ActionStats());
if ( withMetrics )
addServlet(context, "/$/metrics", new ActionMetrics());
+ // TODO Should we support registering other functionality e.g. /$/backup/* and /$/compact/*
Review comment:
Not sure this is possible because `jena-fuseki-main` does not depend on `jena-fuseki-webapp` from which the backup and compact functionality originate.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r517435975
##########
File path: jena-fuseki2/jena-fuseki-webapp/pom.xml
##########
@@ -56,6 +56,12 @@
<artifactId>jena-text</artifactId>
<version>3.17.0-SNAPSHOT</version>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.jena</groupId>
+ <artifactId>jena-tdb2</artifactId>
+ <version>3.17.0-SNAPSHOT</version>
+ </dependency>
Review comment:
Yeah I can probably take this out, I was having some issues with code completion in Eclipse and thought I was missing a dependency
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r517627965
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
It throws `TDBException("Not a switchable TDB database");` so won't that mean it is "500 internal error" not "400 Bad request".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r517917026
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
Well that call is going to happen on the background task thread and any failure will just get logged and not bubble up AFAICT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on pull request #822:
URL: https://github.com/apache/jena/pull/822#issuecomment-728132089
@afs What else should be done to merge this? Are we happy with this as an interim solution until someone has time to revisit the wider implementation?
I can take a look at writing some quick integration tests today but not sure if you wanted any other changes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #822:
URL: https://github.com/apache/jena/pull/822#issuecomment-728160615
I was going on the tick list - let's get something in.
For jena-fuseki-webapp, the code looks OK as it is for this and a good step forward.
We can worry about Fuseki/main later. I have thought a bit about how to add administration functions but not written anything down. It is a chance to do things a bit differently to Fuseki/Webapp, around the fixed area which isn't so convenient for remote administration and also whether it can be simpler and making it opt-in, not always there. But later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse merged pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse merged pull request #822:
URL: https://github.com/apache/jena/pull/822
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r525050174
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
Yeah there's definitely room for some clean up I was trying to figure out a way to add a success/failure flag to tasks but with the multiple layers of wrapping couldn't find a reliable way to get this working
`AsyncPool.submit()` creates a `Callable<Object>`, that wraps a `Runnable`, and then creates and submits an `AsyncTask` which is itself the `Callable<Object>` that actually gets submitted to the executor service
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r526087472
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
If this is done now, could you resolve the ticket? Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #822: Add Fuseki /$/compact/* endpoint (JENA-1987)
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #822:
URL: https://github.com/apache/jena/pull/822#discussion_r517443101
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/main/java/org/apache/jena/fuseki/mgt/ActionCompact.java
##########
@@ -0,0 +1,69 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.fuseki.mgt;
+
+import static java.lang.String.format;
+
+import org.apache.jena.fuseki.ctl.ActionAsyncTask;
+import org.apache.jena.fuseki.ctl.TaskBase;
+import org.apache.jena.fuseki.servlets.HttpAction;
+import org.apache.jena.fuseki.servlets.ServletOps;
+import org.apache.jena.tdb2.DatabaseMgr;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ActionCompact extends ActionAsyncTask
+{
+ public ActionCompact() { super("Compact"); }
+
+ @Override
+ public void validate(HttpAction action) {}
+
+ @Override
+ protected Runnable createRunnable(HttpAction action) {
+ String name = getItemName(action);
+ if ( name == null ) {
+ action.log.error("Null for dataset name in item request");
+ ServletOps.errorOccurred("Null for dataset name in item request");
+ return null;
+ }
+
Review comment:
`DatabaseMgr.compact()` already does this checking so didn't seem relevant to duplicate that here. Either the dataset can be compacted or calling `compact()` will throw a relevant error
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org