SUSE / hanadb_exporter

Prometheus exporter for SAP HANA databases
Apache License 2.0
46 stars 27 forks source link

WIP: use more a functional way for do a pipeline of data modifications #32

Closed MalloZup closed 5 years ago

MalloZup commented 5 years ago

description:

this is a small PR which is imho make the code more maintainable.

In a functional language and way of doing things, you have map, filter, reduce. this can be implemented also in python with list_comprehensions.

I think that if we filter out the filter function from the main loop, we can gain on more comprehnsive code instead of doing it the imperative way, which can be more difficult to maintain. ( cognitive load)

This is a minor refactoring PR, open to review. :grin:

As contra, one could argue that having list_comprehension 3 times can impact the performance but imho I don't think so because the collection is reducing over the 3 iterations and it is not really performance critic.

As you wish, I personally value more the declarative style of programming things then the imperative one.

MalloZup commented 5 years ago

@arbulu89 thx for the time spend on commenting on this.

My 2 cents: I think that in python one can code with a non object oriented approach.

I would not spend my time on arguing in things which I would consider cosmetically important, and take the time from my colleges.

If you have any time, I would recommend you this talk which describe the OOP problems https://www.youtube.com/watch?v=Tb823aqgX_0&feature=youtu.be . ( it is not a language specific talk).

A less object oriented approach can be applied in python3. Objects and classes tangle the way one reason about code, where function require only data and function to act on them. ( but the talk cited is really describing better what I'm saying).

Regarding this PR I agree with you on this point method that is serving http request is not a good idea,, this is correct to me, and can modified, since we are miking a IO side-effect function (impure function) in a for loop when we could just get the results in 1 request and the manipulating the data without http request later.

In any case, thx for you pov, I respect it but I think we could improve drastically the design if we stop to reason about objec/classes and we think about data.

I don't expect that we change drastically now our codebase but I'm pretty much worried of our usage of Classes and OOP in the design in general. ( aka they don't make extensible the design for later) Also object and classes tangle the way about one think about codebases.

Classes were once thought important enough that entire educational curricula were designed around them, and the majority of popular programming languages include dedicated syntax for defining and using them.

But it turns out that classes are often orthogonal to the question of data structure design. Rather than offering us an entirely alternative data modeling paradigm, classes simply repeat data structures that we have already seen:

A class instance is implemented as a dict.
A class instance is used like a mutable tuple.

I cited this from : https://www.aosabook.org/en/500L/contingent-a-fully-dynamic-build-system.htm

I don't expect to change the whole thing now, but we can take this as occasion to reflect about our approaches of doing code, which as developers we want to ask ourself always. (beeing able to use the good technique)

A functional approach in python can be unpopular and not so upstream but we are searching way to do safe code, readable one and extensible code in future without much bug ( aka data driven not top-down).

I'm also currently finding my way of how to do this in python3: examples: https://www.saltycrane.com/blog/2010/02/declarative-vs-imperative-style-python/ But I might need to research also more on this.

So as recap, we definitely have as challenge up to us in this century, to forget about objects and beeing data driven ( but for this is the talk I cited is good).

Beeing that say, I'm pretty happy with the code you wrote in the exporter together with ayoub, it is really good in term of testing coverage and design( even if OOP), documentation. ( it could perhaps be improved in some few places but we are human right :grin: )

My whole message on this pr, is that there are alternative, safer way to code in python or others lang which are more understandable, safer in term of bugs and more extensible.

I will not put any prio to this PR now, but what I'm trying to say is that a function can leave outside of an object where it just manipulate some data without beeing tangled of an object structure.

I think as team we can go in this direction and grow there. As I sayed, I'm ok with the current OOP approach but I see already some downside on it. ( my discussion is not about functional vs oop, but is more which is the technique that allow us to write more safer, code and maintainable)?

For that being sayed a functional approach in python isn't well explained in our industry but it something it is possible and we should aspire to as developers. (i'm trying to find my way to provide a good code for that.)

Consider now this PR as free-time tech debt but isn't that cosmetical the discussion on objects, states and readability top-down approaches vs others. :grin:

I will try to come with more examples on this subject too. Anywyas, thx for review and I would not argue so much if in my opionions you weren't a true good pythonist developer which aim to improve itself, which you arre from my mind. ( but see the video once you have time.. :grin: )

For this pr, wip again

arbulu89 commented 5 years ago

@MalloZup As you said, more than starting a battle between functional and OOP programming (I think both can subexist together and use them depending on your needs) my comments were focused in other things hehe

Mainly, what I don't like about the solution is that it performs actions (filter the lists) that will be repeated every request to the exporter in a quite sensitive part of the execution (http request), when we can perform them at the beginning, and only once, avoiding a lot of not needed code execution. That's why I just suggested other part where the filtering might happen, that's it. Actually, this would lightweight the collect method, that eventually it's a good thing.

MalloZup commented 5 years ago

@arbulu89 yep for that I agree with you that filtering data on https get is bad.