> is there any news regarding this pull request ?
> (https://bitbucket.org/tmidas/midas/pull-requests/30)
apologies for taking a very long time to review the proposed changes.
the main problem with this pull request remains, it tangles together too many changes to the code and I cannot simply
say "this is okey", merge and commit it.
example of unrelated change is diff of mlogger.cxx, change of function in: "db_get_value(hDB, 0, "/Logger/Multithread
transitions" ... )". there is also unrelated changes to whitespace sprinkled around.
can you review your diffs again and try to remove as much unrelated and unnecessary changes as you can?
I could do this for you, and merge my version, but next time you merge base midas, you will have a collision.
unrelated change of function is introduction of something called "downsampling", what is the purpose of this? How is it
different from requesting binned data? Is it just a kludge to reduce the data size? Before we merge it, can you post a
description/discussion to this forum here? (as a separate topic, separate from discussion of PostgreSQL merge).
the changes to add PostgreSQL so fat look reasonable:
- CMakeLists, is always painful but if you do same a MySQL, should be okey, we always end up rejigging this several
times before it works everywhere.
- history.h, ok, minus changes for adding the "downsample" feature
- mlogger.cxx, changes are too tangled with "downsample" feature, cannot review
- SetDownsample() API is defective, should have separate Get() and Set() functions
- history_common.cxx, please do not add downsampling code to history providers that do not/will not support it.
- history_odbc.cxx, please do not change it. it does not support downsampling and never will.
- history.cxx, ditto
- mjsonrpc.cxx, history API is changed, we must know: is new JS compatible with old mhttpd? is old JS compatible with
new mhttpd? (mixed versions are very common in practice). if there is incompatibility, can you recoded it to be
compatible?
- history_schema.cxx: bitbucket diff is a dog's breakfast, cannot review. I will have to checkout your branch and diff
by hand.
changes to mhistory.js appear to be extensive and some explanation is needed for what is changed, what bugs/problems
are fixed, what new features are added.
to move forward, can you generate a pull requests that only adds pgsql to history_schema.cxx, history_common.cxx and
mlogger.cxx and does not add any other functions, features and does not change any whistespace?
K.O.
>
> If you agree to merge I can resolve conflicts that now
> (after two months) are listed...
>
> Regards,
> Gennaro
>
> >
> > Hi,
> > I have updated the PR with a new one that includes TimescaleDB support and some
> > changes to mhistory.js to support downsampling queries...
> >
> > Cheers,
> > Gennaro
> >
> > > > some minutes ago I published a PR for PostgreSQL support I developed
> > > > at INFN-Napoli for Darkside experiment...
> > > >
> > > > I don't know if you receive a notification about this PR and in doubt
> > > > I wrote this message...
> > >
> > > Hi, Gennaro, thank you for the very useful contribution. I saw the previous version
> > > of your pull request and everything looked quite good. But that pull request was
> > > for an older version of midas and it would not have applied cleanly to the current
> > > version. I will take a look at your updated pull request. In theory it should only
> > > add the Postgres class and modify a few other places in history_schema.cxx and have
> > > no changes to anything else. (if you need those changes, it should be a separate
> > > pull request).
> > >
> > > Also I am curious what benefits and drawbacks of Postgres vs mysql/mariadb you have
> > > observed for storing and using midas history data.
> > >
> > > K.O. |