Simplify the interface of NodeDetails by making implementation private

[Samyak2]

The Problem

If you look into utils/load.py, specifically the NodeDetails class, you'll notice that all of the class functions are public but the only function that is actually useful to a user of the class is getDetails. Similarly, some of the class members (ram_pattern and load_pattern) have no use outside of the class, but they are made public.

In python, by default, all members and functions are public. Variables and functions can be made "private" by prefixing the name with an underscore (Note: nothing is truly private in python, these variables can be accessed outside the class but the responsibility of using these in the expected way is unto the user. This is different from languages like Java or C++ where private variables are truly private.)

Why do you need this?

Let's see this with a short story. You, a (first time) contributor to heiko, are working a new scheduler for heiko (you don't need to know what that means atm, but if you're interested, you can find a basic scheduler for heiko implemented here). You need to get details of a node to make some decision (say, to decide which node to run a process on). You make an instance of the class NodeDetails:

node_deets = NodeDetails(node)

Now, to get the actual details from it, you type node_deets. and wait for your editor's code completer to suggest the methods of the class. You see this:

For now you only need the CPU details, so you use getCpuDetails and your editor nicely shows the documentation of it:

So now you need to make an asyncssh connection object, whatever that is. Also, it only returns the output of a command which, in this case, happens to be quite big (try running lscpu -J if you have a linux system and see for yourself). Maybe you notice the parseCpuInfo function which gives only what you need. Maybe you don't notice and write your own parsing function (yikes, now if something is changed inside the getCpuDetails function, you'll have to change this too!). All of this for something that should have abstracted away in the class. This is exactly what is done in the getDetails function which hides all of the asyncssh connection object, calling the functions, etc.. Note that you are not wrong in this case, there were too many functions and variables to go through. It was not easy to do the right thing. Or in other words, it was easy to do the wrong thing.

The getCpuDetails and parseCpuInfo are internal functions used by getDetails. Here, getDetails is the only interface function and the rest are implementation details. Everything outside the class should only use the interface and it is the responsibility of the writer of the class to not change this interface (for example, adding more parameters, changing the return type, etc.). Whereas the implementation could change without affecting the interface. (here is a short example of implementation vs interface in C++).

I also highly recommend watching this amazing talk on designing interfaces - The Most Important Design Guideline by Scott Meyer (don't be fooled by the "lecture" in the title, the talk is quite interesting and you might find it relatable too).

The Solution

Phew, that was a large wall of text. All of this just to say:

• Rename all of the implementation functions and members to prefix them with an underscore.
  • ram_pattern to _ram_pattern
  • load_pattern to _load_pattern
  • getNodeRam to _getNodeRam
  • getCpuUsage ....you get the idea
  • getCpuDetails
  • parserRam
  • parseLoad
  • parseCpuInfo
• Rename in all the places these functions and members are used - all of this should be inside the class!