[gdal-dev] port/md5 cvs_MD5*

[Kurt Schwehr]

Dmitry,

Thank you for doing https://trac.osgeo.org/gdal/changeset/41095

Sorry, I didn't know about the pull request.  I get spammed by github
notifications and have yet to figure out how make the 99.99% I don't care
about go away. :|

I think commits should always reference relevant issues and pull requests.
The commit message here https://trac.osgeo.org/gdal/changeset/41086 was not
particularly useful.  In reading that patch, I assumed that this was a code
brand new to GDAL.  I filter out the changes for drivers that I don't use
and the diffs never go through my repo (0.8M lines locally versus 1.8M in
svn).  The description for r41085 never mentions migrating code from wms to
port or the pr.


On Wed, Dec 20, 2017 at 1:58 PM, Dmitry Baryshnikov <bishop.dev at gmail.com>
wrote:

> Hi Ari,
>
> The pull request and discussion can be found here:
> https://github.com/OSGeo/gdal/pull/280
>
> I cannot imagine that this improvements will affect something else. MD5
> used for cache paths initially, I only added some improvements for WMS
> cache size limits, expire time and split cache per dataset base (i.e. to
> delete cached files when dataset deletes).
>
> What about your idea about common caching classes/functions - this is
> topic to discuss.
>
> Best regards,
>     Dmitry
>
> 21.12.2017 0:32, Ari Jolma пишет:
>
> My comments on this:
>
> 1) The MD5 function seems to be needed only(?) for the WMS cache and
> probably also WFS. To me that highlights the need for some integration work
> regarding caching in GDAL. I also implemented yet another caching code for
> the WCS (without MD5). Looking at my $HOME/.gdal I see also gmlas_xsd_cache
> and srs_cache.
>
> 2) This discussion thread also highlights the need to go towards more pull
> request style development at least for bigger changes.
>
> Ari
>
>
> Kurt Schwehr kirjoitti 21.12.2017 klo 02:56:
>
> Dmitry,
>
> Statements like this indicate a world of trouble:
>
> > I'm not sure that Python produce the same hash than cvs_MD5*.
> > Also I'm not sure what in future they will be the same.
>
> If you remove the swig and python stuff, move the CPLMD5String to
> cpl_md5.{cpp,h}, and follow Even's suggestions, I will be much happier with
> the change.  A C++ test would be good too.  I'll be happy to do a quick fix
> up of a few things that I see in the md5 code once it is to that point.  I
> do think it is a good thing that there be md5 support in port along with
> the existing sha{1,256} code.
>
> I see what you are saying about the OGRGeometry::exportTo{json,kml,gml},
> but that doesn't (to me) support your argument.  My personal opinion is
> that GDAL would have been stronger if those had separate and not been a
> part of OGRGeometry.
>
> If there is a critical difference between CPLMD5String or a language and
> use case where md5 support in that language from GDAL is really required,
> you need to first document that issue.
>
> On Wed, Dec 20, 2017 at 7:46 AM, Dmitry Baryshnikov <bishop.dev at gmail.com
> <mailto:bishop.dev at gmail.com> <bishop.dev at gmail.com>> wrote:
>
>     Just the note that CPLMD5String not only for Python, but any other
>     API clients: C/C++, Java, Perl, etc.
>
>     But, I agree, it can be easily removed.
>
>     Best regards,
>         Dmitry
>
>     20.12.2017 18 <tel:20.12.2017%2018> <20.12.2017%2018>:35, Even
> Rouault пишет:
>
>                 And why are you exposing this to python?  Python has
>                 md5 hashing
>
>         already
>
>                 built in.
>
>             I'm not sure that Python produce the same hash than cvs_MD5*.
>
>         I do hope they return the same thing. MD5 is a standardized
>         algorithm:
>         https://tools.ietf.org/html/rfc1321
>         <https://tools.ietf.org/html/rfc1321>
> <https://tools.ietf.org/html/rfc1321>
>
>         I'm not sure we really need to expose it our CPL MD5 through
>         Python.
>
>
>     _______________________________________________
>     gdal-dev mailing list
>     gdal-dev at lists.osgeo.org <mailto:gdal-dev at lists.osgeo.org>
> <gdal-dev at lists.osgeo.org>
>     https://lists.osgeo.org/mailman/listinfo/gdal-dev
>     <https://lists.osgeo.org/mailman/listinfo/gdal-dev>
> <https://lists.osgeo.org/mailman/listinfo/gdal-dev>
>
>
>
>
> --
> --
> http://schwehr.org
>
>
> _______________________________________________
> gdal-dev mailing list
> gdal-dev at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/gdal-dev
>
>
>
>
>
> _______________________________________________
> gdal-dev mailing listgdal-dev at lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev
>
>
>
> _______________________________________________
> gdal-dev mailing list
> gdal-dev at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/gdal-dev
>



-- 
--
http://schwehr.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20171221/980ae3c1/attachment.html>