Skip to content

Conversation

kuba--
Copy link

@kuba-- kuba-- commented Jun 20, 2018

Closes: src-d/borges#297

  • Listen on closed channel notifications.
  • Retry Publish if we connection dropped in a meantime.

kuba-- added 2 commits June 19, 2018 13:51
Signed-off-by: kuba-- <kuba@sourced.tech>
Signed-off-by: kuba-- <kuba@sourced.tech>
amqp/amqp.go Outdated
@@ -84,6 +85,7 @@ func New(url string) (queue.Broker, error) {
}

b := &Broker{
mut: sync.RWMutex{},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since mut is not a pointer the zero value is already sync.RWMutex{}, so you can remove this line

@@ -13,6 +15,20 @@ import (
"github.com/stretchr/testify/suite"
)

// Pilosa tests require running docker. If `docker ps` command returned an error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Pilosa/AMQP/

Signed-off-by: kuba-- <kuba@sourced.tech>
Copy link
Contributor

@erizocosmico erizocosmico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM when tests pass

@kuba--
Copy link
Author

kuba-- commented Jun 20, 2018

Appveyor build failed (WHAT!?):

Failed to connect to RabbitMQ: dial tcp 127.0.0.1:5672: connectex: No connection could be made because the target machine actively refused it

kuba-- added 2 commits June 20, 2018 22:51
Signed-off-by: kuba-- <kuba@sourced.tech>
Signed-off-by: kuba-- <kuba@sourced.tech>
@kuba--
Copy link
Author

kuba-- commented Jun 21, 2018

Ok, now we can merge.

amqp/amqp.go Outdated
break
}

// b.mut.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need those locks or can we delete the lines? Same in b.chErrors block.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, my bad - leftovers

@jfontan
Copy link
Contributor

jfontan commented Jun 21, 2018

The AppVeyor problem with rabbit happens from time to time. There's already an issue opened to take a look. #10

Signed-off-by: kuba-- <kuba@sourced.tech>
@jfontan jfontan merged commit 41c6976 into src-d:master Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if the network or etcd/rabbit services could be flacky
3 participants