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

Wip/subotic cleanup #35

Merged
merged 7 commits into from
Feb 11, 2016
Merged

Wip/subotic cleanup #35

merged 7 commits into from
Feb 11, 2016

Conversation

subotic
Copy link
Collaborator

@subotic subotic commented Feb 10, 2016

I did some overdue cleanup and documentation

@subotic subotic added this to the On Deck milestone Feb 10, 2016
@subotic subotic added the enhancement improve existing code or new feature label Feb 10, 2016
@benjamingeer
Copy link

Looks good, thanks! Maybe we can also remove Obfuscate.scala?

Can you check these:

  • webapi-production.rst: It talks about running Fuseki in Tomcat, but not about running the built-in Fuseki. Should we still support Fuseki in Tomcat? And do you still want to use supervisord?
  • test-tags.rst: I put a TODO there because I'm not sure it's up to date. Also, now that we use SBT configurations to choose triplestores for tests, does the DbTest tag still make sense?

Could you also add at least a little bit to development-tools.rst, listing the different SBT configurations that are available for different triplestores (fuseki:, graphdb:, etc.), saying that these are defined in KnoraBuild.sbt, and giving examples of xyz:test and xyz:test-only?

@subotic
Copy link
Collaborator Author

subotic commented Feb 10, 2016

Yes, of course. Very good suggestions. Thanks. Maybe you can wait with the merge until I do the other things.

@subotic
Copy link
Collaborator Author

subotic commented Feb 10, 2016

Should we still support Fuseki in Tomcat?

Fuseki under Tomcat is probably the preferred way of running it in production, like running GraphDB under Tomcat. We can have a short documentation describing both cases. But at the end, the system administrator responsible for deployment will need to read/know more than just this short description.

And do you still want to use supervisord?

Good question. Maybe it will be the easiest/fastest way of running it manually at the beginning, at least until we don't have a RPM package and dedicated systemd scripts.

... does the DbTest tag still make sense?

At the moment we probably don't need it. We never really used it. Shall I remove it, or should we leave it? Maybe we will find it useful some day?

@benjamingeer
Copy link

Fuseki under Tomcat is probably the preferred way of running it in production, like running GraphDB under Tomcat. We can have a short documentation describing both cases.

OK, can you write it?

I think it's a good idea to remove unneeded code (less code means less code that can have bugs, and less potential confusion for developers), so let's remove the DbTestTag. I think we could actually use a SipiTestTag instead, because some tests will require Sipi.

@subotic
Copy link
Collaborator Author

subotic commented Feb 11, 2016

Fuseki under Tomcat is probably the preferred way of running it in production, like running GraphDB under Tomcat. We can have a short documentation describing both cases.

OK, can you write it?

Doing it just now.

I have also updated the documentation regarding the section header levels, and going through the whole documentation and adjusting them. Also we talked about the ToDo list a while ago. I added it also.

I think we could actually use a SipiTestTag instead, because some tests will require Sipi.

Then I can rename DbTestTag to SipiTestTag.

@subotic
Copy link
Collaborator Author

subotic commented Feb 11, 2016

What I still didn't cover is the documentation surrounding KnoraBuild.sbt #41 and the different tests that can be called. This will change completely when I make the switch to un-forked testing #16 . I will write the documentation when I introduce this change.

I think we should merge this now as a lot of files where changed. I'm on vacation from tomorrow on and probably wont work on this for more than a week.

@benjamingeer
Copy link

OK, but the wip/subotic_cleanup branch doesn't compile:

/Users/ben/git/Knora/webapi/src/main/scala/org/knora/webapi/store/triplestore/TriplestoreManagerActor.scala:31: object JenaGraphDBActor is not a member of package org.knora.webapi.store.triplestore.embedded
[error] import org.knora.webapi.store.triplestore.embedded.{JenaGraphDBActor, JenaTDBActor}
[error] /Users/ben/git/Knora/webapi/src/main/scala/org/knora/webapi/store/triplestore/TriplestoreManagerActor.scala:31: object JenaGraphDBActor is not a member of package org.knora.webapi.store.triplestore.embedded
[error] import org.knora.webapi.store.triplestore.embedded.{JenaGraphDBActor, JenaTDBActor}
[error]        ^

@subotic
Copy link
Collaborator Author

subotic commented Feb 11, 2016

hmm, not good. I will check.

@subotic
Copy link
Collaborator Author

subotic commented Feb 11, 2016

Sorry, forgot to run clean. Now everything should compile.

benjamingeer pushed a commit that referenced this pull request Feb 11, 2016
@benjamingeer benjamingeer merged commit 1dad337 into develop Feb 11, 2016
@benjamingeer benjamingeer deleted the wip/subotic_cleanup branch February 11, 2016 20:54
benjamingeer pushed a commit that referenced this pull request Feb 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants