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

deprecate LEGACY_REST_URL variable #211

Closed
alexskr opened this issue Sep 7, 2022 · 8 comments · Fixed by #276
Closed

deprecate LEGACY_REST_URL variable #211

alexskr opened this issue Sep 7, 2022 · 8 comments · Fixed by #276

Comments

@alexskr
Copy link
Member

alexskr commented Sep 7, 2022

According to the sample config file LEGACY_REST_URL is 'Legacy REST core service address (BioPortal v3.x and lower)'
BioPortal v3.x rest service was deprecated a while ago but this variable still in use:

url = URI.parse($LEGACY_REST_URL + params[:path])

log = RestClient::Resource.new $LEGACY_REST_URL + "/log", :timeout => 30

@syphax-bouazzouni
Copy link
Contributor

In addition, there are 3 more places where it's used, see the screenshot below.

image

@alexskr
Copy link
Member Author

alexskr commented Sep 7, 2022

flexviz is another deprecated adobe flash based visualization which needs to be purged as well

@syphax-bouazzouni
Copy link
Contributor

syphax-bouazzouni commented Sep 7, 2022

in addition (two), the LEGACY_REST_URL is called in jsonp and remote methods, where they themselves are not used anywhere.

jsonp

image

remote

remote is called inside

def self.add(level, message, request = nil, remote_params = nil)

But will never be called because the add function is called nowhere in the code with request argument

image

my (personnel) conclusion

@syphax-bouazzouni
Copy link
Contributor

syphax-bouazzouni commented Sep 7, 2022

Finally, I have some time ago a list of the unused view (that you can see below), stay to list all the related unused code to these views, and have a second verification of another person.

@alexskr
Copy link
Member Author

alexskr commented Sep 7, 2022

If i understand this correctly log.rb includes remote logging functionality if we set REMOTE_LOGGING. It was previously used for UI to log events to obsolete/non-existing v3 rest API. I think this can be safely removed.

@jvendetti
Copy link
Member

I wasn't able to look at all 37 of the view templates that are listed as unused, but at first glance there are at least a few that are still employed by BioPortal.

layouts/angular.html.erb

This is the layout used by the ontology Browse page:

Started GET "/ontologies" for 127.0.0.1 at 2022-09-07 10:52:24 -0700
Processing by OntologiesController#index as HTML
  Rendering ontologies/browse.html.erb within layouts/angular

layouts/_ontology.html.haml

This layout is referenced inside of the virtual_show method in the notes controller:

render :partial => 'list', :layout => 'ontology'

The method can be triggered by navigating to the top-level Notes tab for any ontology with notes, then clicking on any note subject:

Started GET "/ontologies/BRO/notes/https%3A%2F%2Fdata.bioontology.org%2Fnotes%2Fa5809cba-e4dd-4553-9b24-c010f9d27b50" for 127.0.0.1 at 2022-09-07 14:19:57 -0700
Processing by NotesController#virtual_show as */*
  Parameters: {"ontology"=>"BRO", "noteid"=>"https://data.bioontology.org/notes/a5809cba-e4dd-4553-9b24-c010f9d27b50"}
  Rendered notes/_thread.html.haml (Duration: 1610.9ms | Allocations: 3108739)
Completed 200 OK in 15722ms (Views: 1611.3ms | ActiveRecord: 0.0ms | Allocations: 3473884)

... though it wasn't immediately clear what would trigger the elseif and else clauses inside this method that reference the ontology layout partial.

application/_footer_appliance.html.haml

Used to render a different footer when running the application in appliance mode:

= render partial: "footer_appliance"

notes/_list.html.haml

Used to render the notes associated with a particular class:

Started GET "/ajax_concepts/BRO/?conceptid=http%3A%2F%2Fbioontology.org%2Fontologies%2FResearchArea.owl%23Area_of_Research&callback=load" for 127.0.0.1 at 2022-09-07 14:00:14 -0700
Processing by ConceptsController#show as */*
  Parameters: {"conceptid"=>"http://bioontology.org/ontologies/ResearchArea.owl#Area_of_Research", "callback"=>"load", "ontology"=>"BRO"}
  Ontology Load (0.3ms)  SELECT `ontologies`.* FROM `ontologies` WHERE `ontologies`.`acronym` = 'BRO' LIMIT 1
  ↳ app/helpers/application_helper.rb:367:in `ontolobridge_instructions_template'
  Rendered concepts/_biomixer.html.erb (Duration: 0.1ms | Allocations: 42)
  Rendered concepts/_details.html.haml (Duration: 775.1ms | Allocations: 7617)
  Rendered notes/_list.html.haml (Duration: 6957.3ms | Allocations: 363722)

@syphax-bouazzouni
Copy link
Contributor

layouts/angular.html.erb

This is the layout used by the ontology Browse page:

Started GET "/ontologies" for 127.0.0.1 at 2022-09-07 10:52:24 -0700
Processing by OntologiesController#index as HTML
  Rendering ontologies/browse.html.erb within layouts/angular

Yeah, for the angular layout indeed it is used, I don't remember why I listed it in my notes. Maybe just to remember to remove angular-js and update the browse page.

layouts/_ontology.html.haml

This layout is referenced inside of the virtual_show method in the notes controller:

render :partial => 'list', :layout => 'ontology'

The method can be triggered by navigating to the top-level Notes tab for any ontology with notes, then clicking on any note subject:

Started GET "/ontologies/BRO/notes/https%3A%2F%2Fdata.bioontology.org%2Fnotes%2Fa5809cba-e4dd-4553-9b24-c010f9d27b50" for 127.0.0.1 at 2022-09-07 14:19:57 -0700
Processing by NotesController#virtual_show as */*
  Parameters: {"ontology"=>"BRO", "noteid"=>"https://data.bioontology.org/notes/a5809cba-e4dd-4553-9b24-c010f9d27b50"}
  Rendered notes/_thread.html.haml (Duration: 1610.9ms | Allocations: 3108739)
Completed 200 OK in 15722ms (Views: 1611.3ms | ActiveRecord: 0.0ms | Allocations: 3473884)

... though it wasn't immediately clear what would trigger the elseif and else clauses inside this method that reference the ontology layout partial.

Yes, but I think that the elseif and else clauses inside will never be fulfilled (not 100% sure)

application/_footer_appliance.html.haml

Used to render a different footer when running the application in appliance mode:

= render partial: "footer_appliance"

Ah yes, i found that its only our local code that does no more is it.

notes/_list.html.haml

Used to render the notes associated with a particular class:

Started GET "/ajax_concepts/BRO/?conceptid=http%3A%2F%2Fbioontology.org%2Fontologies%2FResearchArea.owl%23Area_of_Research&callback=load" for 127.0.0.1 at 2022-09-07 14:00:14 -0700
Processing by ConceptsController#show as */*
  Parameters: {"conceptid"=>"http://bioontology.org/ontologies/ResearchArea.owl#Area_of_Research", "callback"=>"load", "ontology"=>"BRO"}
  Ontology Load (0.3ms)  SELECT `ontologies`.* FROM `ontologies` WHERE `ontologies`.`acronym` = 'BRO' LIMIT 1
  ↳ app/helpers/application_helper.rb:367:in `ontolobridge_instructions_template'
  Rendered concepts/_biomixer.html.erb (Duration: 0.1ms | Allocations: 42)
  Rendered concepts/_details.html.haml (Duration: 775.1ms | Allocations: 7617)
  Rendered notes/_list.html.haml (Duration: 6957.3ms | Allocations: 363722)

Indeed,

I updated the list with your remarks, thanks @jvendetti.

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 a pull request may close this issue.

3 participants