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

refactor(ontology): Make knora-admin a separate ontology #1263

Merged
merged 37 commits into from Apr 2, 2019

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Mar 20, 2019

  • Change knora-admin.ttl to http://www.knora.org/ontology/knora-admin.
  • Add KnoraAdmin object to OntologyConstants and move all relevant properties.
  • Make necessary changes to source code so that OntologyConstants works again.
  • Change groups inside permission strings from knora-base:GroupName to knora-admin:GroupName in test data.
  • Update tests.
  • Add upgrade script to update existing repositories.
  • Update docs.

Existing repositories must be updated; see upgrade/1263-knora-admin for instructions.

Resolves #1106.

@subotic
Copy link
Collaborator

subotic commented Mar 20, 2019

cool, thanks 😀

@tobiasschweizer
Copy link
Contributor

@benjamingeer Once you have written the update script, could I try it out?

@benjamingeer
Copy link
Author

@tobiasschweizer

Once you have written the update script, could I try it out?

Yes, I was hoping you would. :) I'll let you know when it's ready.

@benjamingeer benjamingeer changed the title refactor (ontology): Make knora-admin a separate ontology refactor(ontology): Make knora-admin a separate ontology Mar 25, 2019
@benjamingeer benjamingeer mentioned this pull request Mar 26, 2019
@tobiasschweizer
Copy link
Contributor

I'm using GraphDB 8.8, I wonder if that's why.

Graphdb 8.5.0 is the version we use in production due to the memory setting problem.

@benjamingeer
Copy link
Author

I'll try converting knora-test with GraphDB 8.5 and see if I can reproduce the error that way.

@tobiasschweizer
Copy link
Contributor

I'll try converting knora-test with GraphDB 8.5 and see if I can reproduce the error that way.

Ok, thanks a lot!

@tobiasschweizer
Copy link
Contributor

I have just shared another data file on switchdrive with you (data on BEOL test).

@benjamingeer
Copy link
Author

@tobiasschweizer I've reproduced the problem with the Knora test data using GraphDB 8.5. I'll try to find a workaround.

@benjamingeer
Copy link
Author

@tobiasschweizer @subotic I can't get the update to work in SPARQL with GraphDB 8.5. I'm going to take a different approach: dump the contents of the triplestore to files, update the files, then load the files back into the triplestore.

@tobiasschweizer
Copy link
Contributor

I can't get the update to work in SPARQL with GraphDB 8.5. I'm going to take a different approach: dump the contents of the triplestore to files, update the files, then load the files back into the triplestore.

Do you think you can do it in a way so the two dumps (pre and post update) can be diff-ed?

@benjamingeer
Copy link
Author

Do you think you can do it in a way so the two dumps (pre and post update) can be diff-ed?

I think so.

@tobiasschweizer
Copy link
Contributor

Great! Then it is easy to verify whether it worked correctly or not.

@benjamingeer
Copy link
Author

benjamingeer commented Mar 28, 2019

@tobiasschweizer You can try the update script again now. It updates knora-test with GraphDB 8.5 in about 2 minutes. The input and output Turtle files are saved in a temporary directory, and you can look at them, but you can't really diff them, because they're not formatted in quite the same way.

@tobiasschweizer
Copy link
Contributor

Ok, I will check it next week

Copy link
Contributor

@tobiasschweizer tobiasschweizer left a comment

Choose a reason for hiding this comment

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

I can confirm that the data conversion script worked for me. The changes you made look good to me.

Please inform other Knora developers about this upcoming data conversion task.

@benjamingeer
Copy link
Author

@tobiasschweizer Thanks for the review!

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

Successfully merging this pull request may close these issues.

None yet

3 participants