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.

Sign up for GitHub

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

Jump to bottom

DODS/OPeNDAP Driver Removal... Please don't #5173

Closed
vtdrfuka opened this issue Jan 26, 2022 · 11 comments
Closed

DODS/OPeNDAP Driver Removal... Please don't #5173

vtdrfuka opened this issue Jan 26, 2022 · 11 comments
Milestone
3.5.0

Comments

@vtdrfuka
Copy link

Just opening the ticket to request that the DODS driver remains in the mix. We have dependencies on this driver in several CRAN packages and python libraries.

DODS/OPeNDAP
Driver short name: DODS

Driver is considered for removal in GDAL 3.5. You are invited to convert any dataset in that format to another more common one. If you need this driver in future GDAL versions, create a ticket at https://github.com/OSGeo/gdal (look first for an existing one first) to explain how critical it is for you (but the GDAL project may still remove it). T
@rouault rouault added this to the 3.5.0 milestone Jan 26, 2022
@rouault
Copy link
Member

rouault commented Jan 26, 2022 •
edited
Loading

Which driver do you use: the raster one or the vector one ?

One issue we have is that our unit tests (https://github.com/OSGeo/gdal/blob/master/autotest/ogr/ogr_dods.py and https://github.com/OSGeo/gdal/blob/master/autotest/gdrivers/dods.py) don't work anymore due to test servers used initially not being available, and we (at least I) have no idea of how to test that driver and if it still works (the base assumption is that untested code is broken code). Are there public DODS servers somewhere ?
Do you or someone else would volunteer to take over maintenance of those drivers ? At least bringing back the unit tests.
Another annoyance of the raster DODS driver is that it takes directly a URL, which conflicts with the generic HTTP driver. A "DODS:" prefix should be required for dataset names.

@rouault rouault mentioned this issue Jan 26, 2022
Closed
@rouault
Copy link
Member

rouault commented Jan 27, 2022

Call for action to the mailing list audience: https://lists.osgeo.org/pipermail/gdal-dev/2022-January/thread.html#55348

@bradh
Copy link
Contributor

bradh commented Jan 27, 2022

Could the code be moved to the CRAN driver? If it is the only known user, seems possible.

@rouault
Copy link
Member

rouault commented Feb 1, 2022

@vtdrfuka Please provide feedback regarding the questions posted in this ticket

@vtdrfuka
Copy link
Author

vtdrfuka commented Feb 1, 2022

Let me review the test that is failing, as possibly it is just an outdated link. If this is the cause of the removal, it might be within my capability to get it to pass again! I will review a few options and test, with a response within a week; possibly linking the test to the OPeNDAP active development server which has been alive for quite some time.

Also, Thank You for tagging me, for some reason the replies were hidden in my inbox!

@rouault rouault mentioned this issue Feb 6, 2022
Closed
@mdsumner
Copy link
Contributor

mdsumner commented Feb 16, 2022 •
edited
Loading

@vtdrfuka - please share your example sources, perhaps it's better done with NetCDF itself (I haven't used DODS via GDAL for a long time, but NetCDF generally works with URLs in GDAL - and often I disable the DODS driver, or use /vsicurl or NetCDF: to avoid conflation - perhaps we can find alternatives along those lines for you)

@rouault
Copy link
Member

rouault commented Mar 21, 2022

@vtdrfuka can you provide some feedback ? Otherwise the driver will be removed as planned

rouault added a commit to rouault/gdal-feedstock that referenced this issue Apr 1, 2022
@rouault
run_test.sh: remove DODS testing
1813e4c 
I'm unclear why the recipe tests this particular driver... Anyway I'm
about to either remove that driver upstream (cf
OSGeo/gdal#5173), or perhaps less radical
option, just require to have a DODS_RASTER: prefix at the beginning of the
connection string. In any case, if not changing this recipee, this would
break the GDAL upstream CI task that uses this recipee to build Conda package from master.
@rouault rouault mentioned this issue Apr 1, 2022
Merged
gillins pushed a commit to conda-forge/gdal-feedstock that referenced this issue Apr 4, 2022
@rouault
run_test.sh: remove DODS testing (#619)
2879086 
I'm unclear why the recipe tests this particular driver... Anyway I'm
about to either remove that driver upstream (cf
OSGeo/gdal#5173), or perhaps less radical
option, just require to have a DODS_RASTER: prefix at the beginning of the
connection string. In any case, if not changing this recipee, this would
break the GDAL upstream CI task that uses this recipee to build Conda package from master.
@rouault rouault mentioned this issue May 4, 2022
Merged
@rouault
Copy link
Member

rouault commented May 4, 2022

Pull request removing the DODS drivers: #5666

rouault added a commit to rouault/gdal that referenced this issue May 5, 2022
@rouault
Remove deprecated GDAL and OGR DODS/OpenDAP drivers (refs OSGeo#5173, f…
ce5e79f 
…ixes OSGeo#3555)
rouault added a commit that referenced this issue May 5, 2022
@rouault
Merge pull request #5666 from rouault/remove_dods
e7f981d 
Remove deprecated GDAL and OGR DODS/OpenDAP drivers (refs #5173, fixes #3555)
@rouault rouault closed this as completed Aug 5, 2022
@mdsumner
Copy link
Contributor

mdsumner commented Aug 5, 2022

thanks @rouault !

@nyalldawson nyalldawson reopened this Aug 6, 2022
@rouault
Copy link
Member

rouault commented Aug 6, 2022

Why reopening ?

@nyalldawson
Copy link
Collaborator

Hmm, I can't recall doing this -- must have been an errant click!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.5.0
Development

No branches or pull requests
5 participants
@bradh @rouault @nyalldawson @mdsumner @vtdrfuka