Mailing List Archive

On Sun, Dec 12, 2021 at 1:15 AM Peter Bennett <pb.mythtv@gmail.com> wrote:
>
> The old services had various version numbers. See below. In the new
> services all have defaulted to version 1.0. Any recommendations on what
> we should do with the new services?
>
> Set all versions same as the old
> Set them all to 2.0
> Set them all to 20.0
> Set them to previous version + 0.1
> Other?

Well, since you asked, here is an email that I sent to
a dev who asked for a unicasted email regarding the
entire services API issues, which touched upon
API versioning (I do not know if that dev forwarded
my comments to others).....



------


You asked that I send you an email about
the services API. Here it is (be careful what
you ask for, for you may get it?). Obviously
share as you wish.

Here is my recollection of issues I have had
with the services API (at least with the current
version), and some thoughts about improving
things going forward. Some of these I
mentioned a few years ago to the -dev list.
These are in no particular order, and I am
sure I have forgotten some of the issues I
have experienced over the years.

I will admit that some of the ideas imply
a much bigger change than what you or
others have considered for v32.

Gary



* The services API is far too low level. In
most cases they are basically just DB updates.

Ideally, additions/updates/replacements will
consider the needed functionality and not
be just a fancy mysql interface as they are
currently.

* Services API(s) do not implement business logic

To first approximation, none of the Services APIs implement
the business logic that MythTV Setup performs, allowing
the storage of values in database tables that are not
validated/valid, and can lead to issues.

The business logic should be added such that
applications cannot mangle the MythTV database
by accident, or intentionally.

* A number of the variable names are different when
retrieved vs the name that needs to be
set/updated.

I have had to create a translation from the
returned names to the names to be updated
for my apps. Sometimes the difference is
capitalization, sometimes the name is
related, but different (ChanNum vs
ChannelNumber for the Channel service).
Pick a name (any name) and stick with it.

* Service API(s) do not support creating dtv_multiplex entries

While retrieving a Channel definition allows one to
retrieve the dtv_multiplex values, there is no way to
create new multiplexs or to update a dtv_muliplex.
If the Services API is intended to be used to replace
MythTV Setup there needs to be a way to create
dtv_multiplex entries (and likely the dtv_privatetypes)
if the application has it available to store.

* Service API(s) do not support setting of a NULL value

Json based encoding allows the specification of a
form such as {"key": null}. However, the Services APIs
that update database columns such that the column
allows a DB NULL do not store NULL in that column
if passed the null value.

As long as MythTV code differentiates between
NULLs and other values, the Services API needs
to allow those values to be properly evaluated and
stored.

Bonus points, in addition to the bare 'null'
value, there is also 'true' and 'false', and while
most of MythTV stores such as an integer
1 or 0, supporting 'true' and 'false' barewords
might be a good idea for appropriate columns.

* Service API(s) do not store an empty string

Json based encoding allows the specification of a
form such as {"key": ""}, however there are cases
where such are considered as missing, rather than
explicit, and are not actually set (or are set as the
default value, whatever that is).

* The Services APIs should be versioned, and
backward compatibility required for returning only
the values (and setting values) that were in the
requested version. Such backward compatibility
should be maintained for at least one previous
version to allow apps to migrate across supported
versions and master.

Without versioning, certain apps may not be
able to even know that the (new) values they
get are important.

Whether the API itself should take a version
number in the request ...?v19, or the endpoint
URL should be versioned (.../v19/ is up to the
implementer. The important issue for apps
using the services API will continue to work
across at least the supported version and
master.

Admittedly it is not always clear how the
BE service will need to adapt to a previous
versions request, but in cases where the BE
can do a "reasonable" thing, it should.
If the services API cannot support a previous
version (substantial changes, too old, ???)
it should return an appropriate error code
(409?).

Thought experiment, an unversioned query
to determine what version(s) the service
supports so an app can respond according
to any common version that they both support
(or reject the processing)???

* Any Services APIs that take arguments for changes
should leave existing values alone if not replaced

If, at some point in the future, the service API
has an added value, applications may not be
aware of that, and since the services API is
not versioned, it may not return the existing
value, so you may end up with resetting values.

A few of the recent updates to the services API
have done this (good!), but there are a number
of services for which one must return all the
existing values, otherwise they will be updated
with the defaults.

* When one does a AddDBChannel one has to specify
a new (available) ChannelID.

That requires one to first look for an available
number. The ChannelID should be treated as
an opaque value, and auto incremented when
created by AddDBChannel (which should
probably return the ChannelID created?).
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

On Sun, Dec 12, 2021 at 03:02:46AM +0000, Gary Buhrmaster wrote:
> On Sun, Dec 12, 2021 at 1:15 AM Peter Bennett <pb.mythtv@gmail.com> wrote:
> >
> > The old services had various version numbers. See below. In the new
> > services all have defaulted to version 1.0. Any recommendations on what
> > we should do with the new services?
> >
> > Set all versions same as the old
> > Set them all to 2.0
> > Set them all to 20.0
> > Set them to previous version + 0.1
> > Other?
>
> Well, since you asked, here is an email that I sent to
> a dev who asked for a unicasted email regarding the
> entire services API issues, which touched upon
> API versioning (I do not know if that dev forwarded
> my comments to others).....
>
[...]

> * The services API is far too low level. In
> most cases they are basically just DB updates.
>
> Ideally, additions/updates/replacements will
> consider the needed functionality and not
> be just a fancy mysql interface as they are
> currently.

As I recall, the capture card API is one such case. I know I cringed
when I first saw it. It needs a sever overhaul. I'm sure there are
other areas that are the same.

> * Services API(s) do not implement business logic
>
> To first approximation, none of the Services APIs implement
> the business logic that MythTV Setup performs, allowing
> the storage of values in database tables that are not
> validated/valid, and can lead to issues.
>
> The business logic should be added such that
> applications cannot mangle the MythTV database
> by accident, or intentionally.

I put business logic in the recording rule API. The intent was to
make it impossible to create invalid rules.

> * A number of the variable names are different when
> retrieved vs the name that needs to be
> set/updated.
>
> I have had to create a translation from the
> returned names to the names to be updated
> for my apps. Sometimes the difference is
> capitalization, sometimes the name is
> related, but different (ChanNum vs
> ChannelNumber for the Channel service).
> Pick a name (any name) and stick with it.

Yuck. I suppose compatibility could be preserved by changing the the
write APIs to accept the read names too when they differ. That could
cause the internal structs to have both names, though, which would
also be very ugly. Does (Could?) the new, API infrastructure have
support for some type of aliass where the same column could be written
using two or more names?

> * Service API(s) do not support creating dtv_multiplex entries
>
> While retrieving a Channel definition allows one to
> retrieve the dtv_multiplex values, there is no way to
> create new multiplexs or to update a dtv_muliplex.
> If the Services API is intended to be used to replace
> MythTV Setup there needs to be a way to create
> dtv_multiplex entries (and likely the dtv_privatetypes)
> if the application has it available to store.

I'm not sure how to handle dtv_multiplex, and the diseqc tree as well.
My initial reaction has always been that the multiplex parameters
(modulation, frequenty, etc.) should be part of the channel parameters
made visible throught the API. MythTV would then magically manage
dtv_multiplex behind the scene. My fear is that if it's not done
really, really, really well, with copious amounts of business rules,
it might lead to a very messed up dtv_multiplex.

> * Service API(s) do not support setting of a NULL value
>
> Json based encoding allows the specification of a
> form such as {"key": null}. However, the Services APIs
> that update database columns such that the column
> allows a DB NULL do not store NULL in that column
> if passed the null value.
>
> As long as MythTV code differentiates between
> NULLs and other values, the Services API needs
> to allow those values to be properly evaluated and
> stored.
>
> Bonus points, in addition to the bare 'null'
> value, there is also 'true' and 'false', and while
> most of MythTV stores such as an integer
> 1 or 0, supporting 'true' and 'false' barewords
> might be a good idea for appropriate columns.

I don't have a good suggestion. Just maybe use "special" values like
"<null>" when they make sense and "special" boolean columns like
SetOtherClumnNULL=true" when they don't.

> * Service API(s) do not store an empty string
>
> Json based encoding allows the specification of a
> form such as {"key": ""}, however there are cases
> where such are considered as missing, rather than
> explicit, and are not actually set (or are set as the
> default value, whatever that is).

In my latest API changes, which was some time ago, I started checking
a special map variable which indicated which values were actually
supplied. I think that could work. If the map variable says ColumnX
was supplied and it's values is an empty string, then that's what the
user wants.

> * The Services APIs should be versioned, and
> backward compatibility required for returning only
> the values (and setting values) that were in the
> requested version. Such backward compatibility
> should be maintained for at least one previous
> version to allow apps to migrate across supported
> versions and master.
>
> Without versioning, certain apps may not be
> able to even know that the (new) values they
> get are important.
>
> Whether the API itself should take a version
> number in the request ...?v19, or the endpoint
> URL should be versioned (.../v19/ is up to the
> implementer. The important issue for apps
> using the services API will continue to work
> across at least the supported version and
> master.

I don't have a strong preference on which versionaing method to use
other than we should try to be consistent.

> Admittedly it is not always clear how the
> BE service will need to adapt to a previous
> versions request, but in cases where the BE
> can do a "reasonable" thing, it should.
> If the services API cannot support a previous
> version (substantial changes, too old, ???)
> it should return an appropriate error code
> (409?).
>
> Thought experiment, an unversioned query
> to determine what version(s) the service
> supports so an app can respond according
> to any common version that they both support
> (or reject the processing)???
>
> * Any Services APIs that take arguments for changes
> should leave existing values alone if not replaced
>
> If, at some point in the future, the service API
> has an added value, applications may not be
> aware of that, and since the services API is
> not versioned, it may not return the existing
> value, so you may end up with resetting values.
>
> A few of the recent updates to the services API
> have done this (good!), but there are a number
> of services for which one must return all the
> existing values, otherwise they will be updated
> with the defaults.

Agreed. I think I did the following on my latest change:

Read old values from DB.

Apply changes included in the API request.

Sanity check the result and return an error if not sane.

Save the changes if sane.

> * When one does a AddDBChannel one has to specify
> a new (available) ChannelID.
>
> That requires one to first look for an available
> number. The ChannelID should be treated as
> an opaque value, and auto incremented when
> created by AddDBChannel (which should
> probably return the ChannelID created?).

That's an unfortunate choice for doing add. Add should return the new
ID that is automatically created. For backward compatibility, use the
provided ID if the channel doesn't already exist, otherwise, return an
error.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

On Mon, Dec 13, 2021 at 7:27 PM David Engel <david@istwok.net> wrote:

> I suppose compatibility could be preserved

Alternatively, if there is a motivated dev one
could take this (or v33?) as a "once in a lifetime"
opportunity to just throw all the bad approaches
away, and do it right, whatever "right" means,
giving API consumers (and I suspect there are
only a few 3rd parties who have written anything
largish to the existing API, although there is
no doubt a long tail of other(s)) some time to
adapt to an API-NEXT (probably twoish years
based on past release schedules if such a new
design was worked and finalized soonish after
v32 was released). That, of course, presumes
that at least one motivated dev has solved the
ENOTME roblem (perhaps after achieving practical
commercial fusion power delivery, i.e. always at
least three decades away).



> I put business logic in the recording rule API.
....
> Agreed. I think I did the following on my latest change:

Yes, I apologize, as I broad brushed some of
the issues I encountered since I have tended
to remember the pain points, and not the
places where things worked almost exactly
as one would expect/hope.
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org

On 12/11/21 22:02, Gary Buhrmaster wrote:
> * Service API(s) do not support setting of a NULL value
>
> Json based encoding allows the specification of a
> form such as {"key": null}. However, the Services APIs
> that update database columns such that the column
> allows a DB NULL do not store NULL in that column
> if passed the null value.
>
> As long as MythTV code differentiates between
> NULLs and other values, the Services API needs
> to allow those values to be properly evaluated and
> stored.
>
> Bonus points, in addition to the bare 'null'
> value, there is also 'true' and 'false', and while
> most of MythTV stores such as an integer
> 1 or 0, supporting 'true' and 'false' barewords
> might be a good idea for appropriate columns.
On 12/13/21 14:27, David Engel wrote:
> I don't have a good suggestion. Just maybe use "special" values like
> "<null>" when they make sense and "special" boolean columns like
> SetOtherClumnNULL=true" when they don't.
>
Regarding NULL, this may be because of how Qt interprets the NULL value.
https://doc.qt.io/qt-5/qsqlquery.html#bindValue

If you want a boolean: QVariant( ... ).toBool()
https://doc.qt.io/qt-5/qvariant.html#toBool

On Mon, Dec 13, 2021 at 11:43:28PM +0000, Gary Buhrmaster wrote:
> On Mon, Dec 13, 2021 at 7:27 PM David Engel <david@istwok.net> wrote:
>
> > I suppose compatibility could be preserved
>
> Alternatively, if there is a motivated dev one
> could take this (or v33?) as a "once in a lifetime"
> opportunity to just throw all the bad approaches
> away, and do it right, whatever "right" means,
> giving API consumers (and I suspect there are
> only a few 3rd parties who have written anything
> largish to the existing API, although there is
> no doubt a long tail of other(s)) some time to
> adapt to an API-NEXT (probably twoish years
> based on past release schedules if such a new
> design was worked and finalized soonish after
> v32 was released). That, of course, presumes
> that at least one motivated dev has solved the
> ENOTME roblem (perhaps after achieving practical
> commercial fusion power delivery, i.e. always at
> least three decades away).

I wouldn't object for purely aesthetic reasons but I have no real skin
in the game. You, Peter Bennett, the folks who maintain the MythTV
PVR plugin for Kodi and maybe Paul Harrison are the main (only?)
significant, API users that I know of. Discuss among yourselves if
you'd rather see that.

> > I put business logic in the recording rule API.
> ....
> > Agreed. I think I did the following on my latest change:
>
> Yes, I apologize, as I broad brushed some of
> the issues I encountered since I have tended
> to remember the pain points, and not the
> places where things worked almost exactly
> as one would expect/hope.

No offense taken. I understand it is still probably the exception.
Even I've learned some things since I refactored that API belive there
is still some room for improvement.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org