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

Conversation

jedrekfulara
Copy link
Contributor

No description provided.

Copy link
Contributor

@Dref360 Dref360 left a comment

Choose a reason for hiding this comment

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


# 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

{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.

Copy link
Contributor

@Dref360 Dref360 left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@Dref360 Dref360 merged commit f7afc73 into keras-team:master Mar 27, 2018
dschwertfeger added a commit to dschwertfeger/keras that referenced this pull request Apr 6, 2018
…ack-embeddings-from-layer-outputs

* upstream/master: (68 commits)
  fit/evaluate_generator supporting native tensors (keras-team#9816)
  keras-team#9642 Add kwarg and documentation for dilation_rate to SeparableConvs (keras-team#9844)
  Document that "same" is inconsistent across backends with strides!=1 (keras-team#9629)
  Improve tests by designating dtype of sample data (keras-team#9834)
  Add documentation for 'subset' and interpolation' arguments (ImageDataGenerator) (keras-team#9817)
  Revert default theme to readthedocs
  Various docs fixes.
  Fix conflict
  Add support for class methods documentation (keras-team#9751)
  Add missing verbose opt for evaluate_generator (keras-team#9811)
  Added `data_format` to flatten layer. (keras-team#9696)
  Allow saving models directly to binary stream (keras-team#9789)
  Fix ctc_batch_cost() error when batch_size = 1 (keras-team#9775)
  Fix keras-team#9802 (keras-team#9803)
  Fix error in ImageDataGenerator documentation (keras-team#9798)
  fix typo (keras-team#9792)
  keras-team#9733: Extend RemoteMonitor to send data as application/json (keras-team#9734)
  Fixed inconsistencies regarding ReduceLROnPlateau (keras-team#9723)
  Fix doc issue.
  General stateful metrics fixes (keras-team#9446)
  ...
Vijayabhaskar96 pushed a commit to Vijayabhaskar96/keras that referenced this pull request May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants