Populate syncthing errors to the user #1168
Conversation
| @@ -47,7 +47,7 @@ func (s *Syncthing) Monitor(ctx context.Context, disconnect chan error) { | |||
|
|
|||
| // MonitorStatus will send a message to disconnected if there is a synchronization error | |||
| func (s *Syncthing) MonitorStatus(ctx context.Context, disconnect chan error) { | |||
| ticker := time.NewTicker(300 * time.Second) | |||
| ticker := time.NewTicker(30 * time.Second) | |||
There was a problem hiding this comment.
Since we are not calling status API anymore, we can check for errors faster
Codecov Report
@@ Coverage Diff @@
## master #1168 +/- ##
==========================================
+ Coverage 34.54% 34.55% +0.01%
==========================================
Files 65 65
Lines 5440 5438 -2
==========================================
Hits 1879 1879
+ Misses 3358 3355 -3
- Partials 203 204 +1
Continue to review full report at Codecov.
|
pchico83
force-pushed
the
no-space-error
branch
from
ac9f676
to
928c502
Compare
| E: fmt.Errorf("Deployment '%s' has been modified while your development container was active", d.Name), | ||
| Hint: "Follow these steps:\n 1. Execute 'okteto down'\n 2. Apply your manifest changes again: 'kubectl apply'\n 3. Execute 'okteto up' again\n More information is available here: https://okteto.com/docs/reference/known-issues/index.html#kubectl-apply-changes-are-undone-by-okteto-up", | ||
| E: fmt.Errorf("Deployment '%s' has been modified while your development container was active", d.Name), | ||
| Hint: `Follow these steps: |
There was a problem hiding this comment.
how does this look like in windows? I'm not sure if the windows terminal reacts well to this type of break line.
cmd/up/up.go
Outdated
| return errors.UserError{ | ||
| E: err, | ||
| Hint: `The synchrinzation service is running out of space | ||
| Enable persistent volumes in your okteto manifest and try again |
There was a problem hiding this comment.
is this the only way to solve this?
There was a problem hiding this comment.
I think so. do you have anything in mind?
cmd/up/up.go
Outdated
| return errors.UserError{ | ||
| E: err, | ||
| Hint: `Okteto volume is full | ||
| Increase your persistent volume size, run 'okteto down -v' and try 'okteto up' again |
There was a problem hiding this comment.
are we using punctuation in the multi-line error messages? I think we should, since they are more explanation that error message.
There was a problem hiding this comment.
We are using both, I am trying to unify that here. Would you add punctuation on every line?
There was a problem hiding this comment.
I'd use punctuation in the hint, and leave the error as is. specially in these cases where the hint is more of a paragraph.
| @@ -394,7 +393,7 @@ func (s *Syncthing) ResetDatabase(ctx context.Context, dev *model.Dev, local boo | |||
| if err != nil { | |||
| log.Infof("error posting 'rest/system/reset' local=%t syncthing API: %s", local, err) | |||
| if strings.Contains(err.Error(), "Client.Timeout") { | |||
| return errors.ErrUnknownSyncError | |||
| return fmt.Errorf("error resetting syncthing database local=%t: %s", local, err.Error()) | |||
There was a problem hiding this comment.
the 'local=' makes parsing the error harder. can we instead have a function that returns a message with the "local" or "remote" as part of the message?
| return fmt.Errorf("error resetting syncthing database local=%t: %s", local, err.Error()) | |
| return fmt.Errorf("failed to reset syncthing database local=%t: %s", local, err.Error()) |
There was a problem hiding this comment.
we follow this pattern in several places. would you mind opening an issue to track the change?
|
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Signed-off-by: Pablo Chico de Guzman <pchico83@gmail.com>
Signed-off-by: Pablo Chico de Guzman <pchico83@gmail.com>
pchico83
force-pushed
the
no-space-error
branch
from
bd3647c
to
1bd2209
Compare
Signed-off-by: Pablo Chico de Guzman pchico83@gmail.com
Fixes #1127 #1165
Proposed changes