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

NOISSUE - Prevent infinite loop in lora-adapter if Redis init fail #647

Merged
merged 4 commits into from
Mar 14, 2019
Merged

Conversation

manuio
Copy link
Contributor

@manuio manuio commented Mar 13, 2019

Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com

dborovcanin
dborovcanin previously approved these changes Mar 14, 2019
@codecov-io
Copy link

codecov-io commented Mar 14, 2019

Codecov Report

Merging #647 into master will decrease coverage by 0.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #647      +/-   ##
==========================================
- Coverage   87.36%   86.73%   -0.64%     
==========================================
  Files          62       62              
  Lines        3942     3625     -317     
==========================================
- Hits         3444     3144     -300     
+ Misses        340      327      -13     
+ Partials      158      154       -4
Impacted Files Coverage Δ
sdk/go/message.go 40.42% <0%> (-8.19%) ⬇️
http/api/transport.go 89.7% <0%> (-6.22%) ⬇️
readers/mongodb/messages.go 89.18% <0%> (-5.55%) ⬇️
writers/cassandra/init.go 60% <0%> (-4.29%) ⬇️
ws/api/transport.go 87.05% <0%> (-3.99%) ⬇️
users/api/http/transport.go 83.6% <0%> (-3.58%) ⬇️
readers/influxdb/messages.go 87.87% <0%> (-1.75%) ⬇️
writers/influxdb/messages.go 85.5% <0%> (-0.98%) ⬇️
things/api/http/transport.go 98.03% <0%> (-0.42%) ⬇️
readers/cassandra/messages.go 88.88% <0%> (-0.35%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0ed73d...a7b1aa7. Read the comment docs.

cmd/lora/main.go Outdated Show resolved Hide resolved
func (es eventStore) Subscribe(subject string) error {
if err := es.client.XGroupCreateMkStream(stream, group, "$").Err(); err != nil && err.Error() != exists {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't return here. If consumer group already exists, just continue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dusanb94 what we do here? I thought that this is the fix

Copy link
Collaborator

@dborovcanin dborovcanin Mar 14, 2019

Choose a reason for hiding this comment

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

You return only if there is an error other than BUSYGROUP Consumer Group name already exists. If there is no error or error is exactly this one, you will proceed with your subscription.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, this is correct @anovakovic01

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't see that part. IMHO, this line is too long. Can you break it into multiple lines? Also, is there predefined error for this in redis lib?

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 could not find any meaningful error. Line length fixed.

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Copy link
Contributor

@anovakovic01 anovakovic01 left a comment

Choose a reason for hiding this comment

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

LGTM!

@anovakovic01 anovakovic01 merged commit 2f09701 into absmach:master Mar 14, 2019
davide83 pushed a commit to davide83/mainflux that referenced this pull request May 13, 2019
…bsmach#647)

* NOISSUE - Prevent infinite loop in lora-adapter if Redis init fail

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix line length

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
manuio added a commit that referenced this pull request Oct 12, 2020
)

* NOISSUE - Prevent infinite loop in lora-adapter if Redis init fail

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix line length

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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.

4 participants