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

Make multiple attempts to startup rsyslog #164

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

MartinFlores751
Copy link
Contributor

Seems like rsyslog, after being killed, may either start up again by itself, or possibly start up in a delayed manner [1]. Making multiple attempts while also checking if rsyslog has already started up seems to make do for now.


[1] Previous observation when I ran into this issue. Might be incorrect or irrelevant.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Nice. Let's get some comments in here and I think it'll be good to go

irods_testing_environment/irods_setup.py Outdated Show resolved Hide resolved
irods_testing_environment/irods_setup.py Outdated Show resolved Hide resolved
Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Changes look fine to me. If/Once this is merged, we should capture the TODO in an issue (not before it's merged though).

This is just personal preference so feel free to ignore ... Consider removing the square brackets from around the attempt counter. It's a little shocking to see #[1] instead of #1.

Let's give @alanking a chance to think about this before adding the pound.

@MartinFlores751
Copy link
Contributor Author

Fair on that, it was kind of an "Oh hey, we use '[]' when we throw in variables" sort of thing. Perhaps [#1] would be better if we want to keep that sort of style?

@MartinFlores751
Copy link
Contributor Author

Also, should we mention issue #163 in the commit message? Or is that a separate thing?

@korydraughn
Copy link
Contributor

Fair on that, it was kind of an "Oh hey, we use '[]' when we throw in variables" sort of thing. Perhaps [#1] would be better if we want to keep that sort of style?

Sure. That works too.

Also, should we mention issue #163 in the commit message? Or is that a separate thing?

Yes. That is exactly the thing we are providing a temporary fix for.

I'm hoping the RHEL9 people or Rsyslog people provide the true solution in the near future.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Looks good. Let's resolve open conversations, squash to one commit, and consider linking to #163(?). Leave off # for now.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Looks good. Please add #!

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.

4 participants