Task API feedback: areas for improvement

[jsteenb2]

backstory: I went on a wild ride yesterday trying to integrate task updates into pkger. I wasted a lot of time/effort on this endeavor and that experience I think deserves an issue so we are aware of some of sharp edges we have consuming task API via procedural calls.

that experience resulted in this PR: #17810

There are a number of things I noticed with a list of potential remedies:

• the http.TaskService does not satisfy the influxdb.TaskService,
  making it impossible to use as a dependency if expecting a influxdb.TasksService

  • rename it to something else to remove the obvious consequences of not satisfying the
  • make it satisfy the interface (nuke the http specifc output types)

• the APIs for create and update do not share common types. For example:
  creating a task takes every field as a string, but in the update it is
  taken as a options.Duration type. A step further and you'll notice that
  create does not need an option to be provided, but the update does. Its
  jarring trying to understand the indirection here. I struggled mightily
  trying to make sense of it all with the indirection and differing types.
  Made for a very difficult task (no pun intended) when it should have been
  trivial. Opportunity here to fix these up and make this API more uniform
  and remove unnecessary complexity, like the options type and making types
  consistent.

  • make the API congruent, use the same types across the entire domain
  • remove options or make it optional. If it is pulled from query, then let me be the source of truth. This is hard both in PC and RPCs alike. Very painful to work with.

• Nested IDs that get marshaled, are no bueno when you want to marshal a task
  that does not have an ID in it, for either user/org/or self IDs. Its a challenge
  just to do that. This is a pressing issue for MANY resources in our platform.

  • don't marshal/unmarshal influxdb.IDs directly
  • remove validation from marshal json. That should be an opt in not forced on the user (imo 😬)

• Lots of logs in the kv.Task portion where we hit errors and log and others where
  we return. It isn't clear what is happening. The kv implementation is also very
  procedural, and I found myself bouncing around like a ping pong ball trying to
  make heads or tails of it.

  • leave (better) comments at the very least
  • provide some higher level direction somewhere on why some error are returned up the callstack and some are not

• There is auth buried deep inside the kv.Task implementation that kept throwing me
  off b/c it kept throwing errors, instead of warns. I assume, not sure if I'm
  correct on this, but that the stuff being logged is determined inconsequential
  to the task working. I had lots of errors from the auth buried in there, and hadn't
  a clue what to make of it.... this is more a general design concern. Auth shouldn't bleed
  into every laayer of the callstack.

  • general design concern, don't drop auth beyond http/auth middleware

• find API is inconsistent. This issue single handily cost me hours of productivity. Hitting the
  API with a name, sometimes works. When using the name, it often doesn't find it for w/e
  reason. When providing the org id alongside it, it would definitely be returned. I could not
  make sense of this looking through the code with the pounding headache it gave me. This
  may very well be a bug. TBH... I don't quite know b/c I can't figure otu the intention of the
  design. This is an issue all over the place, but with tasks it is a little more unobvious than
  in other spots.

  • Whatever the constraints are, they need to be documented properly and exposed in a way that is obvious.
  • Preferably this would be programmatically enforced