Skip to content
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

JENA-979: add a fuseki admin service to list all existing backups #82

Closed
wants to merge 3 commits into from

Conversation

yyz1989
Copy link

@yyz1989 yyz1989 commented Jun 26, 2015

No description provided.

@yyz1989 yyz1989 changed the title Jena 979 add a fuseki admin service to list all existing backups JENA-979: add a fuseki admin service to list all existing backups Jun 26, 2015
@rvesse
Copy link
Member

rvesse commented Jun 29, 2015

+1 to the idea

The implementation seems to violate REST principles though

POST should be used for changing data so should not be supported for a service that only provides information

HEAD should be used only to determine what the resource supports so should not return a body and should only return information about supported methods and content-types

@afs
Copy link
Member

afs commented Jun 29, 2015

I disagree a bit about POST - yes, it's not nice REST but it is pragmatically useful to ensure up-to-date information (the client can force it even if the server or proxies decide to cache).

HEAD - completely agree. I think the body may be chopped anyway but I'd need to check.

@afs
Copy link
Member

afs commented Jun 29, 2015

/documentation/fuseki2/fuseki-server-protocol.html will need updating but it is quite a small change.

Collections.sort(fileNames);
for (String str : fileNames) {
builder.value(str);
}
Copy link
Member

Choose a reason for hiding this comment

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

Given Java 8, this is a little briefer as fileNames.forEach(builder::value).

Copy link
Author

Choose a reason for hiding this comment

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

I think we are based on Java 7, no?

Copy link
Member

Choose a reason for hiding this comment

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

No, I believe we are now officially at Java 8 for the main line of development. We use Java 8-only idioms elsewhere. In any case, it's nothing to worry about. I just have a vicious yen for brevity. :)

@ajs6f
Copy link
Member

ajs6f commented Jun 29, 2015

POST is one way to enable a "force up-to-date`, but it should also be possible to supply an ETag, which is a little more to the exact need. Perhaps Fuseki has helpers for this?

@afs
Copy link
Member

afs commented Jun 29, 2015

Indeed - etags are one example of a server driven mechanism. They need more machinery than this PR has though. HTTP headers are not always easy to deal with in client libraries, either access or setting the conditional GET headers. (And not all developers are aware of or are interested in details (burden) of all HTTP features.)

POST is the easy way for a client to force a live view. I think we should leave it in because it is near zero cost. Fuseki has the same elsewhere.

@ajs6f
Copy link
Member

ajs6f commented Jun 29, 2015

Okay-- do you think it's worth a separate ticket and PR for some helpers for ETags, so that future new Fuseki HTTP functions could offer them at low cost? I don't mean block this PR, I mean a totally separate ticket.

@afs
Copy link
Member

afs commented Jun 29, 2015

Here, the client browser is an admin interface.The read-information operations are not a performance pain-point.

Helpers presume how they are being used in Fuseki is defined and it's not. Each use may be quite different. It's not just how to use E-Tags but deciding what they represent so look at in context.

e.g. JENA-626 (query caching) is a case where they would be useful. And how would even Jena's own client library deal with them.

@ajs6f
Copy link
Member

ajs6f commented Jun 29, 2015

I meant something as simple as a method that accepts an identifier and response and adds the correctly-formatted tag.

@afs
Copy link
Member

afs commented Jun 29, 2015

We are agreed that it's not related to this PR. Let's discuss it on dev@ and not tangle this PR with that discussion.

@ajs6f
Copy link
Member

ajs6f commented Jun 29, 2015

Okay, sorry to cloud the waters.

@yyz1989
Copy link
Author

yyz1989 commented Jun 29, 2015

Yeah, I agree with Rob and Andy. I do think accepting POST is necessary, but I apologize for my negligence for HEAD. That is due to my laziness because I took the main structure from another service directly :D

@afs
Copy link
Member

afs commented Jul 15, 2015

WIP: I'm incorporating this by integrating into the admin action framework (i.e. extends ActionCtl)

Currently:

  • The URL is /$/backup-list Camel-case is less common for URL API naming (YMMV).
  • Returns the file names, not the full path names. The server does not reveal its absolute path setup.
  • Returns non-hidden files : Filters on !file.isHidden() && file.isFile()
  • Response is Content-Type: application/json

@asfgit asfgit closed this in 2ab1ca8 Jul 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants