Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply master (v1) latest version to v2 #157

Merged
merged 15 commits into from
Jan 6, 2021

Conversation

gabrielcorado
Copy link
Contributor

@gabrielcorado gabrielcorado commented Jan 4, 2021

The following is being ported: #139, #143, #148, #151, #150 and #154.

The latest addition to master (#153) is not being ported in this PR. It will require some changes and will be done in a separate PR.

rodopoulos and others added 14 commits September 22, 2020 13:38
When agent write goroutine closed the channel heartbeat and send functions
could still write to the channel causing a panic. send already had a recover
for this but heartbeat didn't have. This commit does not close the channel anymore
and to make so the writers don't block sending when an agent is closed a select is used
reading from die channel that signals that a agent is closed.

The write channel is never closed but it's ok as when a Agent is not used anymore it will be
garbage collected as the agent is the only reference to it.
…lose-logs

Reduce connection close error logs
* Add more information to nats timeout error logs
processRemote function is always called with a nil server and because of this
pitaya was crashing in production. This commit moves the log from this process
to rpc call function that has the target information and returns this info
wrapping the error returned.
@@ -68,6 +69,12 @@ func (m *MyComp) RemoteErr(ctx context.Context) (*test.SomeStruct, error) {

type unregisteredStruct struct{}

func errorIs(t *testing.T, err1 error, err2 error) {
if !assert.ObjectsAreEqual(err1, err2) && !errors.Is(err1, err2) {

Choose a reason for hiding this comment

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

Shouldn't it be !assert.ObjectsAreEqual(err1, err2) || !errors.Is(err1, err2)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we assume that the struct being equal is not enough to consider it as the same (same for the opposite). But @victor-carvalho has more context on it.

Choose a reason for hiding this comment

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

Gotcha. Just double checking, anyway 😂

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1092

  • 32 of 48 (66.67%) changed or added relevant lines in 9 files are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.2%) to 67.279%

Changes Missing Coverage Covered Lines Changed/Added Lines %
group.go 2 3 66.67%
kick.go 1 2 50.0%
cluster/etcd_service_discovery.go 5 7 71.43%
agent/agent.go 10 15 66.67%
service/remote.go 4 11 36.36%
Files with Coverage Reduction New Missed Lines %
agent/agent.go 2 78.91%
cluster/etcd_service_discovery.go 7 80.24%
Totals Coverage Status
Change from base Build 1059: -0.2%
Covered Lines: 4799
Relevant Lines: 7133

💛 - Coveralls

@victor-carvalho
Copy link
Contributor

LGTM

Copy link
Contributor

@rodopoulos rodopoulos left a comment

Choose a reason for hiding this comment

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

LGTM

@gabrielcorado gabrielcorado merged commit b86b664 into v2 Jan 6, 2021
@gabrielcorado gabrielcorado deleted the feature/v2-apply-master-changes branch January 6, 2021 14:09
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.

None yet

7 participants