←back to thread

421 points briankelly | 3 comments | | HN request time: 0.653s | source
Show context
necovek ◴[] No.43575664[source]

The premise might possibly be true, but as an actually seasoned Python developer, I've taken a look at one file: https://github.com/dx-tooling/platform-problem-monitoring-co...

All of it smells of a (lousy) junior software engineer: from configuring root logger at the top, module level (which relies on module import caching not to be reapplied), over not using a stdlib config file parser and building one themselves, to a raciness in load_json where it's checked for file existence with an if and then carrying on as if the file is certainly there...

In a nutshell, if the rest of it is like this, it simply sucks.

replies(23): >>43575714 #>>43575764 #>>43575953 #>>43576545 #>>43576732 #>>43576977 #>>43577008 #>>43577017 #>>43577193 #>>43577214 #>>43577226 #>>43577314 #>>43577850 #>>43578934 #>>43578952 #>>43578973 #>>43579760 #>>43581498 #>>43582065 #>>43583922 #>>43585046 #>>43585094 #>>43587376 #
1. theteapot ◴[] No.43579760[source]

> to a raciness in load_json where it's checked for file existence with an if and then carrying on as if the file is certainly there...

It's not a race. It's just redundant. If the file does not exist at the time you actually try to access it you get the same error with slightly better error message.

replies(1): >>43594949 #
2. necovek ◴[] No.43594949[source]

There is a log message that won't be output in that case: whether getting a full, "native" FileNotFound exception is better is beside the point, since the goal of the code was obviously to print a custom error message.

And it's trivial to achieve the desired effect sanely:

  try:
      with open(...) ...

  except FileNotFound:
      logger.error(...)
      raise

It'd even be fewer lines of code.

replies(1): >>43607565 #
3. theteapot ◴[] No.43607565[source]

Or even fewer by doing it in a global exception handler instead of every time you try to open a file, since all your doing is piping the error though logger.