Remove docs mocks for msgpack and psutils

[cachedout]

Also make docs build py3 compat

[cachedout]

Go Go Jenkins!

[cachedout]

Go Go Jenkins!

[rallytime]

YAY!

[whiteinge]

I get that Py3 doesn't want/need the binary flag but Windows needs it, right?. If so, how do you target Py2, Py3, and Windows all together?

[whiteinge]

msgpack and psutils were mocked for a reason. Mocking is decidedly not the ideal solution, but it's what we have to avoid having to install a lot of deps just to build the docs. (We're in need of an alternate to mocking, perhaps using Salt's loader but that's a separate discussion.)

We can certainly decree a given subset of deps are required but there should be a reason for the items on that list. One of the problems with mocking is adding/removing libs can often lead to a wild-goose-chase where it stops uncovering one problem but also introduces a new one.

The build system in its current form was written to build without msgpack and psutil so this change introduces new import errors. I would like to have discussed this change before merging. We can adjust the build system if needed but I'd like to discuss what problem this fixed and whether this fix is the right fix for that.

[cachedout]

Hi @whiteinge

Thanks for the feedback here.

I took a look at a couple alternatives to this and I'll outline each below:

1. Extending the current mock system to support return values.

I experimented with rewriting the mock system inside Sphinx's configuration file to allow it to return arbitrary values which could be drawn from a map of object being mocked -> type of value to return

The problem with this approach is that it's really just kicking the can down the road. If I mock msgpack to return an int and at some point in Salt, we call a function in that package that returns a tuple, it breaks again.

2) Modifying Salt itself. It may be possible to catch exceptions like these in Salt itself which would allow the docs build to continue without a problem but I really hate doing this. (I didn't test this, FWIW.)

3) Making these sorts of errors non-fatal in the Sphinx build. This requires more Sphinx knowledge than I personally have, if it's even possible at all.

I'm happy to revert this and discuss a more substantial fix but I definitely fall in the camp of thinking that it's reasonable to require the same hard deps to build Salt docs as it takes to install Salt.

[whiteinge]

1. Agreed about the can. Mocking is necessarily a fragile cat-and-mouse game. Deps are added that break the build and then mocks are added to fix them. As mentioned, less than ideal but I opted to go this route for a use-case.
2. Agreed; non-starter.
3. Autodoc works in the way you might expect, by import'ing the module and inspect'ing the bits. So module-level effects will get executed and tracebacks mean you can't inspect that module.

No need to revert IMO. The build was broken already so it's trading one traceback for another.

Mocking was added in the very, very early days before we controlled our own build system and even before the try...except ImportError was an established pattern in modules. Earlier today @jacobhammons and I discussed requiring the regular Salt deps to build and he's interested to give it a try. It will be interesting to see how many mocks we can do without nowadays.

[whiteinge]

@jacobhammons you asked about the list of required deps. The libs in base.txt are a good starting point. You may or may not also need the libs in zeromq.txt.

[isbm]

@whiteinge @cachedout At least with the #35763 I can now happily build docs for our package and control the mocking details.

[whiteinge]

Thank you for the fix! Responding to the discussion here and in your PR:

We've talked in Slack and meatspace about this but I filed an issue to centralize the discussion: #35883. I hope @jacobhammons doesn't mind that I filed it on his behalf.