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

[DAPHNE-#867] Change ConfigParser::fileExists() behavior to not throw #867

Closed

Conversation

corepointer
Copy link
Collaborator

I consider this a bugfix but I'll run it through a PR to raise attention to the change. Tests are running fine (after fixing the one that tests the previous behavior). If there were no concerns raised by the time CI ran through I'll merge it right in.

Changing the behavior of fileExists() to a boolean operation as suggested by the method's name. Throwing an exception us up to the caller of this method.

Closes #867

@corepointer corepointer added the bug A mistake in the code. label Oct 18, 2024
@corepointer corepointer added this to the v0.4 milestone Oct 18, 2024
corepointer added a commit to corepointer/daphne that referenced this pull request Oct 18, 2024
…not throw

Changing the behavior of fileExists() to a boolean operation as suggested by the method's name. Throwing an exception us up to the caller of this method.

Closes daphne-eu#867
@corepointer
Copy link
Collaborator Author

The same semantic problem exists with keyExists() imho. Bit I'll leave that one for future work as my main concern to change this in the first place was a failing distributed test that did not find its config file.

Copy link
Collaborator

@philipportner philipportner left a comment

Choose a reason for hiding this comment

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

Not sure if you wanted to close a related issue but you put the PR# itself in the description :)

…not throw

Changing the behavior of fileExists() to a boolean operation as suggested by the method's name. Throwing an exception us up to the caller of this method.

Closes daphne-eu#867
@corepointer
Copy link
Collaborator Author

Not sure if you wanted to close a related issue but you put the PR# itself in the description :)

Since issues and pull requests share the same sequence number, I frequently abuse this to have a ticket number and a place for discussion right here in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A mistake in the code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants