This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Don't use logrus Fatalf() method #630

Closed

invidian opened this issue Jun 16, 2020 · 4 comments

Labels

Member

invidian commented Jun 16, 2020

Fatalf() after printing given message makes process exit with code 1, which can be used instead of using return statements in the function, which in my opinion hides the complexity of the functions. We should make functions return errors and then we can call Fatalf() once.

The text was updated successfully, but these errors were encountered:

invidian mentioned this issue

Add Admission webhook to lokomotive #704

Merged

invidian mentioned this issue

Refactor backend handling #828

Open

Member

johananl commented Aug 25, 2020

Fatal()/Fatalf() is typically used only in the topmost level of an application. Any imported package or library shouldn't decide to terminate the process on behalf of the caller.

In the topmost level of Lokomotive (which is cli/cmd) I find it OK to use Fatal()/Fatalf() since the alternative would be print + exit, which is longer.

We should make functions return errors and then we can call Fatalf() once.

Which functions for example are we talking about?

Member Author

invidian commented Aug 25, 2020

In the topmost level of Lokomotive (which is cli/cmd) I find it OK to use Fatal()/Fatalf() since the alternative would be print + exit, which is longer.

Assuming that we should almost have no code in cli/cmd, I agree 😄

Member Author

invidian commented Aug 25, 2020

Which functions for example are we talking about?

I'd say all functions which can be/should be unit tested.

invidian added the technical-debt label

invidian mentioned this issue

Create code style guide for Go, Terraform and Helm #883

Open

invidian mentioned this issue

Decouple Lokomotive versioning from lokoctl #312

Open

invidian added a commit that referenced this issue


          cli/cmd: don't use contextLogger.Fatalf for cluster apply and destroy

112d745

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Refs #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initialize() return error instead of using Fatalf

fb8d5fd

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initializeTerraform() return error

a7e2222

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make clusterExists return error

428459e

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make upgradeComponent() return error

7389f4a

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

1df0744

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initializeTerraform() return error

6aad8d1

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make clusterExists return error

af78b7e

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make upgradeComponent() return error

fa540ad

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

1a19041

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

de37ebb

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

bdecc4b

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

45afd6f

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

9bb3e86

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

6127d0b

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initialize() return error instead of using Fatalf

3aa6283

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initializeTerraform() return error

57221f3

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make clusterExists return error

736bbd4

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make upgradeComponent() return error

cccfa49

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

51bcff7

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initialize() return error instead of using Fatalf

3e85f8d

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initializeTerraform() return error

f21dc89

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make clusterExists return error

b5ce34b

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make upgradeComponent() return error

e8432f6

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

661e419

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

ac07419

To soften the dependency on log.Entry and to separate it strictly
from *cobra.Command, so it is easier to move this code around in the
future.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initialize() return error instead of using Fatalf

b5a599b

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make initializeTerraform() return error

ac1dfa0

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make clusterExists return error

8fc5016

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Also, it no longer requires logger, so argument has been removed.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make upgradeComponent() return error

cdf5f77

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component apply use errors instead of Fatalf

e707440

To soften the dependency on log.Entry and to separate it strictly
from *cobra.Command, so it is easier to move this code around in the
future.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

449dd57

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

15fbb07

To remove a dependency on log.Entry to make moving this code around
easier and to avoid hiding function complexity from logger.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

8f02bf8

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

20ced84

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

fa617c5

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

230e793

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

43b3897

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

bef5e2a

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

3b1b201

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

0ab7920

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

865e529

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

bbde4e3

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

knrt10 pushed a commit that referenced this issue


          cli/cmd: make component render-manifest use errors instead of Fatalf

11d5bc3

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

knrt10 pushed a commit that referenced this issue


          cli/cmd: make health return error instead of using Fatalf

517c9af

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

knrt10 pushed a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

3a3a5e2

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

knrt10 pushed a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

7a3aebc

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: knrt10 <kautilya@kinvolk.io>

knrt10 added a commit that referenced this issue


          cli/cmd: make component delete use errors instead of Fatalf

daed8cb

To soften the dependency on log.Entry and to separate the functionality
from CLI code. This also makes code easier to move around and avoids
hiding function complexity.

Part of #630

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Co-authored-by: knrt10 <kautilya@kinvolk.io>

Member Author

invidian commented Nov 2, 2020

This is now done. We only use Fatalf now where it actually make sense.

invidian closed this as completed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.