Allow import when LDAP is down

[daradib]

While we could use some more complicated logic or warn when _get_first_available_uid is broken, likely because LDAP is down, for simplicity we just pass and handle errors later during account creation.

[chriskuehl]

Do you think it makes sense to move this logic into the function body?

The more I work with large Python codebases, the more I become convinced that import-time side-effects are a smell. They have a lot of problems:

• You might not even want to use this function, but you're forced to incur the cost on import if you use anything in this module.
• Rapid iteration is really hard because just importing the code takes a long time.
• Difficult to mock import-time code in tests.
• Some linters want to import code and might fail.
• Profiling and debugging slow execution is more difficult because the time expense is hidden far away from the consumer

In general I think we should try to have importing code be "free". A lot of functions do some kind of caching when called, and while that has some of the same problems (especially wrt mocking), I think it's more manageable. If we're worried about performance regressions on the first call, we could add warmup code to create.

[chriskuehl]

This scenario is also an example where import-time side-effects produce some fairly unexpected behavior. None of us thought about this case when writing or reviewing the code, or the effects it could have on ocflib's users.

[matthew-mcallister]

I want to suggest that not only should this not be moved into the function body, it should not even be stored anywhere in the module. create_account should take a cache or known_uid kwarg, or be available as a method of a class, so that code calling create_account can do its own caching.

In general, consider that any code which repeatedly calls some LDAP- or other service-dependent ocflib function, such as create_account, is basically a long-lived daemon which either has a pre-existing cache or might as well have its one (create in this case, though ocfweb fits the bill as well); other code like a script only gets run once and doesn't benefit from the cache anyways. If create weren't the only code creating accounts, it would also make sense for any code using this cache to update it periodically instead of only when a new account needs to be created, and this would only happen every so often, not hundreds of times a minute. Hence, this isn't a library optimization in the vein of memoization as much as it is a substitute for a more sophisticated caching solution.

Maybe this is over complicated considering how tiny the scale is here, but I believe (perhaps naively) that doing things the scalable and side-effect-free way is usually the way to go. Since ocflib is a library, not a framework, it would be nice for ocflib to just contain the minimal logic necessary to carry out the tasks it was designed for.

[chriskuehl]

@matthew-mcallister sounds good to me

[fawaf]

fixes https://github.com/ocf/ocfweb/issues/103

[matthew-mcallister]

@daradib Sorry for stealing your PR.

[daradib]

No worries this was a quick workaround, thanks for a more general solution.