Use platform.release instead of platform.dist in env_platfroms.py

[vglaidler]

This will break Pandokia functionality that we are not currently using. Raise an exception with a helpful hint in case anyone does try to use it.

(Argh, simple approach is insufficient. WIP.)

[vglaidler]

I'm only surprised bc I don't see any place where the line of code in question yesterday is removed or commented out (something.dist()). Is that change elsewhere?

It's inside the class constructor that now raises an exception instead of executing.

[vglaidler]

@oiintam I am unsure how best to proceed here. I was focusing on the piece of code that uses platform.dist(), but after I implemented "platform is just broken now because we don't think we use it", I saw lots of other places where it was imported (but by that point had to stop working & did not analyze the uses.).

What do you think is the best path forward?

[oiintam]

Hmm, I was wondering if we keep the affected area in that script instead of everywhere that uses platform especially since only one function in platform package is deprecated now.

From the code of env_platform.py, it looks like platform.dist() is only used for getting the operating system version. If so, platform has another method called version() and that maybe a simpler substitution.

[vglaidler]

From the code of env_platform.py, it looks like platform.dist() is only used for getting the operating system version. If so, platform has another method called version() and that maybe a simpler substation.

That sounds promising, can you please go ahead & make that change on this branch?

[oiintam]

can you please go ahead & make that change on this branch?

Sure thing.

[oiintam]

Actually, reading more about the code, it looks like it is trying to get the release number instead of version, which are not necessary the same thing. So I used another method from platform package called release() instead.

[oiintam]

Manual testing result seems good:

[vglaidler]

that looks great @oiintam . What is the best way to test it in normal pandokia use?

Last time we needed to make a pandokia change, we decided to just merge it and then if it breaks, revert it. Does that still make sense as a plan?

[oiintam]

Does that still make sense as a plan?

Yes, I think it makes sense.

[oiintam]

What is the best way to test it in normal pandokia use?

Tbh, I am not very sure which party of pandokia uses this. 🤔

[vglaidler]

Tbh, I am not very sure which party of pandokia uses this. 🤔

Well, I think it is only used if we have a pandokia_environment file with a platform section.... but tests are better than inferences. :)

What I meant above was, let's make sure this change doesn't break anything we normally do. :)

[oiintam]

Hmm, should we merge this?

[vglaidler]

Hmm, should we merge this?

👍 - tho I don't know if @cdsontag would rather we wait until he's got loon properly sorted out, and/or the TP change is confirmed to be fully working, in case this breaks something too.

(tl;dr: yes based on the PR, when based on tests returning to stable)

[cdsontag]

waiting on this for a day or so, yes, that would be ideal

[oiintam]

Okay, thanks for the fast feedback! :) I will wait till tomorrow to merge this then.

[oiintam]

I am merging this today since loon is okay now.