Misc fixes

[Flamefire]

I encountered a test failure locally and started to look around the code with pylint enabled which started to highlight some more bugs. So I decided to select a few checks from pylint which seem reasonable and fix them.

I ended up with pylint -j 4 -d all -e undefined-variable,reimported,unused-import,singleton-comparison,bad-indentation,bad-super-call,arguments-differ,unused-variable spython which now exits clean and could be used on CI.

Some bugs found:

• Missing includes causing crashs on error paths
• Changed environment in test causing other tests to fail
• Missing parameters
• Accidentally unused variables
• "Pythonic" issues (super call parameters wrong, idents, overwritten function, ...)

See individual commits for details. Roughly each commit corresponds to a pylint check.

[vsoch]

Would you care to add the pylint to the testing, so in the future we can avoid these errors?

[vsoch]

Thanks for these fixes @Flamefire, let's do a little more upkeep to finalize the PR. Please:

• Address the questions above
• Add pylint to the testing (done first so we fail early)
• Fix the test error with SPYTHON_SINGULARITY_VERSION
• Bump the version

[Flamefire]

@vsoch I got a failure from the inspect command. It seems singularity 2 returns an enclosing data object. Besides that 2 and 3 are the same. Wouldn't it be better to unify the output and remove the enclosing data? Otherwise you'd need branching whether on 2 or 3

[Flamefire]

I'd like you to do what you think is best

I changed the inspect output as proposed: a5d7b0e and made the split_uri function: 9d18cd8
I also renamed the property uri to protocol and removed the trailing :// to have it uniform over all classes: 915721b

But I do think the implementation is still wrong/not as intended. See tests in fae8cbb. I don't know enough about the internals and intended use so I'd suggest you take this as a base line and change it so that it matches your expectations.

[vsoch]

Yes it's been incredibly challenging maintaining support for Singularity 2.* vs 3.* - the entire image group command has gone away, and the data structures returned for something like inspect are different.

[vsoch]

I just fixed the merge conflicts.

[vsoch]

You've done a lot of changes, a lot of criticism, and it's a bit much for one pull request. Please scope this PR down to a list of changes, and any current concerns that you have. We talked about the recipe parser needing changes, and now you are saying there are substantial issues here as well? I need you to be more specific about your concerns if you expect me to look into them.

[vsoch]

If you want me to pick up, I'll pull your fork and start working on it in a separate PR.

[vsoch]

Pull request will be continued here! #116 @Flamefire all of your commits are represented there. On the other thread, could you describe your remaining concerns with this PR? The Instance object is still useful for Singularity 3.+, but the previous "ImageClient" that was associated with an image command group was deprecated after 2.6. I don't see users ever instantiating it - it's just a means to interact with the previous image group commands.

[vsoch]

@Flamefire the lack of data was an error introduced into Singularity during the port from 2 to 3, see here sylabs/singularity#2790. They haven't fixed it yet. We would need to do a check for the attribute, or just return the entire thing. I'll take a look at this now.