launcher icon indicating copy to clipboard operation
launcher copied to clipboard

Table exec's should use `tablehelpers.Exec`

Open directionless opened this issue 2 years ago • 4 comments

We have a couple places we create tables by parsing the output from execs. These should all use our helper. This achieves several goals:

  • Uses a standard pattern
  • Enforces timeouts
  • Logs errors

We should:

  • Find all cmd.Run invocations and replace them with tablehelpers.Exec (there may be some exceptions. execwrapper and build generators for example)
  • Add a forbidigo rule to lint for them

directionless avatar Aug 30 '23 22:08 directionless

I think this really means pushing the dataflatten stuff to use tablehelpers.Exec

directionless avatar Feb 14 '24 20:02 directionless

I think this had been blocked on adding WithUid functionality to our exec helper, and that's present now. 🎉

directionless avatar Apr 19 '24 13:04 directionless

If I understand correctly, the issue suggests that we lint using forbigo to ensure that code with in the /ee/tables dir only uses tableHelpers.Exec instead of directly calling Run on commands from allowedcmd. Currently golantci-lint does not support applying specific rules to specific paths in a folder. So far the only solution I have found would involve a second execution of golangci-lint that points to a different config that would only run on this specific path. I'm not sure that is worth the overhead.

James-Pickett avatar May 01 '24 15:05 James-Pickett

We should evaulate it in other places. Either exclude, or move to the helper, depending

directionless avatar May 01 '24 15:05 directionless