One question: what will “get_platform()” return when it’s an actual Cray & when it’s Cray simulator? I think it’ll be useful to specify the values in the design doc.
And do we really need all three interfaces? It seems a bit of an overkill, one can just do: if get_platform() == “Cray” instead of calling is_cray() right? So, maybe just get_platform() is enough?
get_platform() returns string as “cray” if it’s a actual Cray cluster and returns string as “craysim” if it’s a Cray simulator.
I have updated and mentioned these two string values in doc.
Talking about three interfaces, if test user wants to just check whether the platform is Cray or Cray simulator then he can just call “is_cray()” or “is_cray_simulator()” for any specified system in a cluster, which will return true or false. User doesn’t need to remember strings (‘cray’, ‘craysim’) and compare it, which will simplify users job in my opinion. Also in very rare case if we change these strings internally then it will affect at all places.
Hi @borlesanket,
Do you see a need for a test writer to call “get_platform”?
If not, I think you should NOT expose “get_platform” as an interface, and only expose the 2 new interfaces that you expect test writers to use: “is_cray” and “is_cray_simulator”.
Because paraphrasing what @agrawalravi90 said these three interfaces seems like a bit of overkill.
Hi @lisa-altair,
I don’t see any need for a test writer to call “get_platform”, he or she can just call “is_cray” and “is_cray_simulator”. I have removed get_platform interface and updated the design doc as per requirement.
Hi @mkaro,
Yes, path to cname means “/proc/cray_xt/cname”, but as replied above to Lisa, I have removed “get_platform” interface, which was having these paths mentioned.
Thanks @borlesanket. Now that you have removed get_platform, it would be good to remove the references to it in the details section of the remaining interfaces (e.g. “Call DshUtils.get_platform(),”). That is implementation detail.
Other than that, it looks good.
@borlesanket
The PTL has an interface get_platform() to detect platform. You can update the existing get_platform() for new platforms and it will suffice your requirements.
get_platform() API document should say what all platform it can detect. I think we don’t need new interface for each platform.