PMM-7 Don't panic in case of path to encryption file doesn't exist. #3307
Conversation
BupycHuk
requested review from
idoqo and
JiriCtvrtka
and removed request for
a team
| defaultEncryptionMtx.Unlock() | ||
| defer defaultEncryptionMtx.Unlock() | ||
| DefaultEncryption, err = New() | ||
| if err != nil { |
There was a problem hiding this comment.
do you think we need to panic? it will be returned by stack to main
There was a problem hiding this comment.
Pardon me, I believed
if err != nil {
return err
}
return nil
was an equivalent of return err.
Is it not?
| @@ -81,21 +80,22 @@ func New() *Encryption { | |||
| case os.IsNotExist(err): | |||
| err = e.generateKey() | |||
| if err != nil { | |||
| logrus.Panicf("Encryption: %v", err) | |||
| return nil, err | |||
There was a problem hiding this comment.
Is it make sense to continue if PMM is not able to generate encryption key?
There was a problem hiding this comment.
We can't run tests locally because of this panic
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #3307 +/- ##
==========================================
- Coverage 43.53% 43.50% -0.04%
==========================================
Files 366 366
Lines 44162 44161 -1
==========================================
- Hits 19228 19213 -15
- Misses 23254 23266 +12
- Partials 1680 1682 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
PMM-7
Link to the Feature Build: SUBMODULES-0
If this PR adds or removes or alters one or more API endpoints, please review and add or update the relevant API documents as well:
If this PR is related to some other PRs in this or other repositories, please provide links to those PRs: