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.

Sign up for GitHub

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

Jump to bottom

Support direct upload/download to Image I/O daemon on oVirt node #35

Merged
merged 6 commits into from
May 25, 2020
Merged

Support direct upload/download to Image I/O daemon on oVirt node #35

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f281406
Support direct upload/download to Image I/O daemon on oVirt node
ganto May 2, 2020
af575d4
ovirt_disk: auto-detect proxy connection based on engine version
ganto May 2, 2020
36446c7
Use proxy as fallback
mnecas May 21, 2020
7c90057
add create_transfer_connection
mnecas May 21, 2020
c6da921
correct warn msg
mnecas May 21, 2020
0fdab53
update transfer names
mnecas May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion plugins/modules/ovirt_disk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ def create_transfer_connection(module, transfer, context, connect_timeout=10, re
connection.connect()
except OSError as e:
# Typically ConnectionRefusedError or socket.gaierror.
module.warn("Cannot connect to %s, trying %s: %s", transfer.transfer_url, transfer.proxy_url, e)
module.warn("Cannot connect to {}, trying {}: {}"
.format(transfer.transfer_url, transfer.proxy_url, e))
Copy link
Member

Choose a reason for hiding this comment

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

Formatting messages for logs is bad idea. Formatting is expensive and should be done only when the log message is actually logged. For warnings this is usually the case, but for debug message it is always not and having 2 ways to log in the same code is bad.

I hope this is not the way ansible scripts use logging. If it is I guess we want to keep this consistent with other logs here.

Copy link
Member

@mnecas mnecas May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is not for logging it just shows a user warning message.
And around formatting, I don't mind to use it as it was before.


url = urlparse(transfer.proxy_url)
connection = HTTPSConnection(
Expand Down