Remove controllers/datastore.py #362
Comments
|
Thanks @rgrp that would be great. We'll also need to remove the routes [1] and probably some tests. [1] https://github.com/okfn/ckan/blob/master/ckan/config/routing.py#L189 |
rufuspollock
added a commit
that referenced
this issue
Feb 16, 2013
…S) datastore. Note, have not removed (and will not remove) references to old datastore in: * templates_legacy (assume these will get removed anyway) * i18n (assume this will get rebuilt prior to release)
rufuspollock
added a commit
that referenced
this issue
Feb 16, 2013
…tore (by removing api_data method in package controller).
The ultimate change was reasonably substantial - removal of an entire method on
a controller and associated template. It began simply with the need to
remove/update reference this routing call in api_data method on package
controller
url = h.url_for('datastore_read', id=id, qualified=True)
I spent about half an hour trying to understand if this method was used at all.
I ultimately determined that we had a setup where clicking on the Data API
button in the resource view had 2 possible effect routes:
* Via href={url} => package.py/api_data method =>
template/package/resource_api_data.html =>
template/ajax_snippets/api_info.html
* Via JS intercept of click => api/snippets controller =>
template/ajax_snippets/api_info.html
But there was a subtle difference: the first route (which is *not* normally
followed since JS is usally enabled) still uses the old datastore URL (this is
why I'd started this investigation!).
(Aside: this was hard to debug through the interface as JS is enabled and I
note we do not have resource view tests so the non AJAX route is not tested).
I first tried to correct this by setting the URL to the correct datastore API
but then discovered other parameters were missing - it seems the signature of
the call to ajax_snippets template has changed but that the relevant code in
package/resource_data_api.html has not been updated:
{% snippet 'ajax_snippets/api_info.html', datastore_root_url=datastore_root_url, embedded=true %}
Rather than continue fixing I decided at this point to remove this non-AJAX
fallback code in its entirety.
Comment: this was a subtle bug that has crept in because there are now 2 ways to do
something and one of these routes (the non-AJAX route) is almost never used ...
I wonder whether it is necessary always to have both options (AJAX and
non-AJAX) as it likely to multiply code complexity and, hence, these sort of bugs.
tobes
added a commit
that referenced
this issue
Feb 18, 2013
amercader
added a commit
that referenced
this issue
Feb 20, 2013
rufuspollock
added a commit
that referenced
this issue
Feb 20, 2013
…S) datastore. Note, have not removed (and will not remove) references to old datastore in: * templates_legacy (assume these will get removed anyway) * i18n (assume this will get rebuilt prior to release)
rufuspollock
added a commit
that referenced
this issue
Feb 20, 2013
…tore (by removing api_data method in package controller).
The ultimate change was reasonably substantial - removal of an entire method on
a controller and associated template. It began simply with the need to
remove/update reference this routing call in api_data method on package
controller
url = h.url_for('datastore_read', id=id, qualified=True)
I spent about half an hour trying to understand if this method was used at all.
I ultimately determined that we had a setup where clicking on the Data API
button in the resource view had 2 possible effect routes:
* Via href={url} => package.py/api_data method =>
template/package/resource_api_data.html =>
template/ajax_snippets/api_info.html
* Via JS intercept of click => api/snippets controller =>
template/ajax_snippets/api_info.html
But there was a subtle difference: the first route (which is *not* normally
followed since JS is usally enabled) still uses the old datastore URL (this is
why I'd started this investigation!).
(Aside: this was hard to debug through the interface as JS is enabled and I
note we do not have resource view tests so the non AJAX route is not tested).
I first tried to correct this by setting the URL to the correct datastore API
but then discovered other parameters were missing - it seems the signature of
the call to ajax_snippets template has changed but that the relevant code in
package/resource_data_api.html has not been updated:
{% snippet 'ajax_snippets/api_info.html', datastore_root_url=datastore_root_url, embedded=true %}
Rather than continue fixing I decided at this point to remove this non-AJAX
fallback code in its entirety.
Comment: this was a subtle bug that has crept in because there are now 2 ways to do
something and one of these routes (the non-AJAX route) is almost never used ...
I wonder whether it is necessary always to have both options (AJAX and
non-AJAX) as it likely to multiply code complexity and, hence, these sort of bugs.
tobes
added a commit
that referenced
this issue
Feb 20, 2013
amercader
added a commit
that referenced
this issue
Feb 20, 2013
|
This appears to me in master so closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This file (I believe) was for the original ES based datastore and is no longer relevant (we kept for v1.8) https://github.com/okfn/ckan/blob/master/ckan/controllers/datastore.py
It should now be removed. I volunteer to do this if it is agreed this should be removed.
The text was updated successfully, but these errors were encountered: