-
Notifications
You must be signed in to change notification settings - Fork 10
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
Parse custom net box URI (Unix socket) correctly #323
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ylobankov
force-pushed
the
ylobankov/fix-net-box-uri
branch
from
September 5, 2023 12:30
38298c1
to
afc702f
Compare
ylobankov
force-pushed
the
ylobankov/fix-net-box-uri
branch
2 times, most recently
from
September 6, 2023 18:22
07bd6cf
to
f162f45
Compare
ylobankov
changed the title
Consider login and password in net box URI
Parse custom net box URI (Unix socket) correctly
Sep 6, 2023
ylobankov
force-pushed
the
ylobankov/fix-net-box-uri
branch
2 times, most recently
from
September 6, 2023 21:54
e548d64
to
0cb27d9
Compare
Totktonada
reviewed
Sep 7, 2023
Totktonada
reviewed
Sep 7, 2023
Totktonada
reviewed
Sep 7, 2023
Totktonada
approved these changes
Sep 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objections from my side (after a brief glance).
ylobankov
force-pushed
the
ylobankov/fix-net-box-uri
branch
2 times, most recently
from
September 7, 2023 12:41
4ce8ef8
to
f337a1f
Compare
Until this moment, luatest didn't parse a custom net box URI correctly in the case of a Unix socket if it was as follows: unix/:/tmp/foo1/bar1.iproto login:password@unix/:/tmp/foo2/bar2.iproto So it created wrong directories for such sockets: unix/:/tmp/foo1/ login:password@unix/:/tmp/foo2/ instead of correct ones: /tmp/foo1/ /tmp/foo2/ Now this is fixed. Fixes #313
I don't like creating files, directories, whatever at the point of the 'class instance' initialization -- the `new` function. It's more logical and gently to create all required resources on the start.
The conditions like if a == nil and b == nil then c = 'something' end if a == nil and b then c = 'something else' end going in a row look a little strange. This patch groups similar conditions from the `Server:initialize` function into the following construction: if a == nil then if b == nil then c = 'something' else c = 'something else' end end
ylobankov
force-pushed
the
ylobankov/fix-net-box-uri
branch
from
September 7, 2023 12:51
f337a1f
to
49a74b2
Compare
Totktonada
approved these changes
Sep 7, 2023
ochaplashkin
approved these changes
Sep 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Until this moment, luatest didn't parse a custom net box URI correctly
in the case of a Unix socket if it was as follows:
So it created wrong directories for such sockets:
instead of correct ones:
Now this is fixed.
Fixes #313