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

#9733: Extend RemoteMonitor to send data as application/json #9734

Merged
merged 4 commits into from
Mar 27, 2018
Merged
Changes from 1 commit
Commits
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
16 changes: 12 additions & 4 deletions keras/callbacks.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,25 +547,29 @@ class RemoteMonitor(Callback):
Events are sent to `root + '/publish/epoch/end/'` by default. Calls are
HTTP POST, with a `data` argument which is a
JSON-encoded dictionary of event data.
If send_as_json is set to True, the content type of the request will be application/json.
Otherwise the serialized JSON will be send within a form

# Arguments
root: String; root url of the target server.
path: String; path relative to `root` to which the events will be sent.
field: String; JSON field under which the data will be stored.
headers: Dictionary; optional custom HTTP headers.
send_as_json: whether the request should be send as application/json
Copy link
Contributor

Choose a reason for hiding this comment

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

add the type to be consistent
send_as_json: Boolean, ...

"""

def __init__(self,
root='http://localhost:9000',
path='/publish/epoch/end/',
field='data',
headers=None):
headers=None, send_as_json=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

skip a line to be consistent

super(RemoteMonitor, self).__init__()

self.root = root
self.path = path
self.field = field
self.headers = headers
self.send_as_json = send_as_json

def on_epoch_end(self, epoch, logs=None):
if requests is None:
Expand All @@ -580,9 +584,13 @@ def on_epoch_end(self, epoch, logs=None):
else:
send[k] = v
try:
requests.post(self.root + self.path,
{self.field: json.dumps(send)},
headers=self.headers)
if self.send_as_json:
payload = {self.field: send} if self.field else send
Copy link
Contributor

Choose a reason for hiding this comment

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

this if is not documented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dref360 There is this fragment in the class documentation:
If send_as_json is set to True, the content type of the request will be application/json.
Otherwise the serialized JSON will be send within a form

Do you want me to add something more?

Copy link
Contributor

Choose a reason for hiding this comment

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

field can be None and it would also work. But it's not documented in the field doc

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also inconsistent with line 593

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dref360 the field param makes sense only when the data is sent within the form (the old path), there is no necessity to wrap the data when it is sent as application/json, so I've now simplified the logic and updated the docs.

requests.post(self.root + self.path, json=payload, headers=self.headers)
else:
requests.post(self.root + self.path,
{self.field: json.dumps(send)},
headers=self.headers)
except requests.exceptions.RequestException:
warnings.warn('Warning: could not reach RemoteMonitor '
'root server at ' + str(self.root))
Expand Down