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

Use save_file for external repositories #1763

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

phyrog
Copy link
Contributor

@phyrog phyrog commented Aug 8, 2016

This fixes #1535. Until #1761 is merged, it also exhibits the bug from #1753.

I'm not sure if I can delete save_file_only completely from the code, or if it is needed for the tests (which is the only place it is still used).

@ebolloff ebolloff assigned ebolloff and eugenk and unassigned ebolloff Aug 10, 2016
@eugenk
Copy link
Member

eugenk commented Aug 10, 2016

I suggest to rebase on staging (#1761 is merged) and then refactor the specs such that save_file_only can be removed. Instead of save_file_only, we could use save_file(..., do_parse: false). The do_parse flag is actually not needed, because we only parse if the filename has the extension of an ontology...

@phyrog phyrog force-pushed the 1535-ontologies_in_external_repositories_are_not_parsed branch from 177f7df to c7a91f9 Compare August 10, 2016 07:44
@phyrog
Copy link
Contributor Author

phyrog commented Aug 10, 2016

I added the do_parse: false just to be more explicit.

@eugenk
Copy link
Member

eugenk commented Aug 10, 2016

I don't know if we should fix it in this branch or treat it as a new issue:

The ontology that is put into the external repository gets a URL or Loc/Id that does not allow to inspect the ontology itself. Instead, the file view is shown.

How to test: Try to upload https://raw.githubusercontent.com/eugenk/test-import-dol/master/Importer1.dol and then check the ontology that is put into the repository "External"

Edit: Parsing this ontology also fails with

no input found for: http://localhost:3000/ref/2/external/github.com/eugenk/test-import-dol/raw/master/OwlBase.owl

so this should be handled in this PR.

@eugenk
Copy link
Member

eugenk commented Aug 10, 2016

Another issue: The ontology in the external repository has two versions right from the beginning. This is probably because we use both ExternalRepository#create_ontology and Repository#save_file inside of ExternalRepository#add_to_repository.

@phyrog
Copy link
Contributor Author

phyrog commented Aug 10, 2016

I can't seem to reproduce either of those bugs locally on my machine.

The ontologies get the locids /external/Importer1 and /external/Importer1//Importer1 and have only one version. They are also properly analyzed by hets (status is done).

@eugenk
Copy link
Member

eugenk commented Aug 10, 2016

Weird: I just retried the whole process and now I get a blank page when I want to access the ontology in the external repository. The log says that the URL is routed to ontologies#show.

@eugenk
Copy link
Member

eugenk commented Aug 12, 2016

When I upload https://raw.githubusercontent.com/eugenk/test-import-dol/master/Importer1.dol to the repository "default"
And I wait for the parsing to finish
Then I get

[4] pry(main)> Repository.find_by_path('external').ontologies.first.versions.count
=> 2
[5] pry(main)> Repository.find_by_path('external').ontologies.select(%i(name basepath 
=> [#<Ontology name: "Owlbase", basepath: "github.com/eugenk/test-import-dol/raw/master/OwlBas...", file_extension: ".owl">]
[7] pry(main)> Repository.find_by_path('external').ontologies.first.locid
=> "/external/github.com/eugenk/test-import-dol/raw/master/OwlBase.owl"

@eugenk
Copy link
Member

eugenk commented Aug 12, 2016

When I visit this Loc/Id (http://localhost:3000/external/github.com/eugenk/test-import-dol/raw/master/OwlBase.owl), I see to the file view instead of the ontology view.
This is because the Loc/Id still has the file extension.

Edit: No, this time I get a 406 (Not acceptable) code

@phyrog
Copy link
Contributor Author

phyrog commented Aug 12, 2016

The locid is generated this way because the node in the dgraph has the name http://github.com/eugenk/test-import-dol/raw/master/OwlBase.owl. Is this the correct output, and if yes, can I safely remove the extension in every case?

@eugenk
Copy link
Member

eugenk commented Aug 12, 2016

@tillmo, what's best to do here?

The ontology already has a different URL than the original one because it's hosted on ontohub. So, I think it's safe to use a name and a Loc/Id without the file extension for the imported ontology.

@phyrog phyrog force-pushed the 1535-ontologies_in_external_repositories_are_not_parsed branch from bbfb417 to 59611bb Compare August 13, 2016 09:35
@phyrog
Copy link
Contributor Author

phyrog commented Aug 13, 2016

More problems:
When the imported ontology includes a class (and probably also with others), this is what hets returns for the symbol (already parsed with Nokogiri):

{
  "kind"=>"Class",
  "name"=>"<https://github.com/eugenk/test-import-dol/raw/master/OwlBase#OwlBaseClass>",
  "iri"=>"https://github.com/eugenk/test-import-dol/raw/master/OwlBase#OwlBaseClass",
  "text"=>"Class <https://github.com/eugenk/test-import-dol/raw/master/OwlBase#OwlBaseClass>"
}

This results in a locid like

/external/github.com/eugenk/test-import-dol/raw/master/OwlBase//%3Chttps%3A%2F%2Fgithub.com%2Feugenk%2Ftest-import-dol%2Fraw%2Fmaster%2FOwlBase%23OwlBaseClass%3E

instead of just

/external/github.com/eugenk/test-import-dol/raw/master/OwlBase//OwlBaseClass

I could do something similar to Ontology#locid_for_child, but I don't know if this is without risk of breaking other ontologies.

For comparison, what hets normally returns:

{
  "kind"=>"Class",
  "name"=>"American",
  "iri"=>"http://www.co-ode.org/ontologies/pizza/pizza.owl#American",
  "label"=>"Americana",
  "text"=>"Class American"
}

@tillmo
Copy link
Member

tillmo commented Aug 13, 2016

for now, this might be a solution. However, the deeper problem is a Hets problem, see spechub/Hets#1596

@phyrog
Copy link
Contributor Author

phyrog commented Aug 13, 2016

The problem does not seem to be the the locid itself, but rather the router. I created a new ticket here: #1790.

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.

Ontologies which are in the External repository are not parsed
4 participants