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

adds repeats to try fix len(dns). Issue79 #252

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

DanielPerkins7
Copy link
Contributor

resolve #79

DanielPerkins7 and others added 2 commits August 2, 2023 14:37
resolve apel#79 . i have just looped everything in the try statement although i am not sure if it is whats needed.
@DanielPerkins7 DanielPerkins7 requested a review from a team as a code owner August 2, 2023 14:56
ssm/agents.py Fixed Show fixed Hide fixed
fails = 0
while fails < 3 and len(dns) == 0:
for line in lines:
if line.isspace() or line.strip().startswith('#'):
Copy link
Contributor

Choose a reason for hiding this comment

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

The lines inside the for loop should be indented 4 spaces.

if len(dns) == 0:
fails = 0
while fails < 3 and len(dns) == 0:
for line in lines:
Copy link
Contributor

@rowan04 rowan04 Aug 3, 2023

Choose a reason for hiding this comment

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

is lines still being defined here?

# If no valid DNs, SSM cannot receive any messages.
if len(dns) == 0:
fails = 0
while fails < 3 and len(dns) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

fails is never being incremented, so will never reach 3

@rowan04
Copy link
Contributor

rowan04 commented Aug 3, 2023

Also, could these changes be rebased into one commit? You might need Tom to help you :)

resolve apel#79 . i have just looped everything in the try statement although i am not sure if it is whats needed.
@norealroots
Copy link

Also, could these changes be rebased into one commit? You might need Tom to help you :)

Okay, I have tried - but I am at a loss as to why this isn't working. I thought I had it, but it's just added another commit and merge.

@rowan04
Copy link
Contributor

rowan04 commented Aug 4, 2023

Also, could these changes be rebased into one commit? You might need Tom to help you :)

Okay, I have tried - but I am at a loss as to why this isn't working. I thought I had it, but it's just added another commit and merge.

huh, weird. maybe don't worry about it then?

@tofu-rocketry tofu-rocketry self-assigned this Aug 4, 2023
@tofu-rocketry tofu-rocketry added this to the 3.5.0 milestone Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Receiver crashes if DNs file is being written to and read from at the same time.
4 participants