Skip to content

Commit 8e852e5

Browse files
authored
HParams: Update tb_http_client to support the hparams plugin backend (#6337)
## Motivation for features / changes In #6318 I ported over the runs_data_source from internal tensorboard in order to begin fetching hparams data in the timeseries dashboard. However, I later learned that it didn't play well with internal Colab which does not allow POST requests and thus had to revert the change in #6336. ## Technical description of changes The current implementation of this "hackaround" converts post bodies to query params and assumes that all post bodies are of type `FormData`. This does not work in this situation for two reasons: 1) HParams Plugin backend does not accept `FormData` 2) The [the hparams plugin](https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/hparams/hparams_plugin.py#L191) only expects GET requests to have a query parameter titled "request" which is a serialized JSON object. ## Screenshots of UI changes N/A ## Alternate designs / implementations considered I considered modifying the hparams plugin to accept many query parameters but decided that the logic required to convert them to a single JSON object (including nested objects/arrays and parsing numbers) was messier than the just doing it on the client.
1 parent f83272f commit 8e852e5

File tree

2 files changed

+62
-14
lines changed

2 files changed

+62
-14
lines changed

tensorboard/webapp/webapp_data_source/tb_http_client.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,20 @@ function convertFormDataToObject(formData: FormData) {
4545
return result;
4646
}
4747

48+
function bodyToParams(body: any | null, serializeUnder?: string) {
49+
if (!body) {
50+
return;
51+
}
52+
const params =
53+
body instanceof FormData ? convertFormDataToObject(body) : body;
54+
if (serializeUnder) {
55+
return {
56+
[serializeUnder]: JSON.stringify(params),
57+
};
58+
}
59+
return params;
60+
}
61+
4862
export const XSRF_REQUIRED_HEADER = 'X-XSRF-Protected';
4963

5064
/**
@@ -84,7 +98,8 @@ export class TBHttpClient implements TBHttpClientInterface {
8498
path: string,
8599
// Angular's HttpClient is typed exactly this way.
86100
body: any | null,
87-
options: PostOptions | undefined = {}
101+
options: PostOptions | undefined = {},
102+
serializeUnder: string | undefined = undefined
88103
): Observable<ResponseType> {
89104
options = withXsrfHeader(options);
90105
return this.store.select(getIsFeatureFlagsLoaded).pipe(
@@ -100,7 +115,7 @@ export class TBHttpClient implements TBHttpClientInterface {
100115
if (isInColab) {
101116
return this.http.get<ResponseType>(resolvedPath, {
102117
headers: options.headers ?? {},
103-
params: convertFormDataToObject(body),
118+
params: bodyToParams(body, serializeUnder),
104119
});
105120
} else {
106121
return this.http.post<ResponseType>(resolvedPath, body, options);

tensorboard/webapp/webapp_data_source/tb_http_client_test.ts

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,51 @@ describe('TBHttpClient', () => {
9292
});
9393
});
9494

95-
it('converts POST requests to GET when in Colab', () => {
96-
const body = new FormData();
97-
body.append('formKey', 'value');
98-
store.overrideSelector(getIsFeatureFlagsLoaded, true);
99-
store.overrideSelector(getIsInColab, true);
100-
tbHttpClient.post('foo', body).subscribe(jasmine.createSpy());
101-
httpMock.expectOne((req) => {
102-
return (
103-
req.method === 'GET' &&
104-
req.urlWithParams === 'foo?formKey=value' &&
105-
!req.body
106-
);
95+
describe('converts POST requests to GET when in Colab', () => {
96+
it('using form data', () => {
97+
const body = new FormData();
98+
body.append('formKey', 'value');
99+
store.overrideSelector(getIsFeatureFlagsLoaded, true);
100+
store.overrideSelector(getIsInColab, true);
101+
tbHttpClient.post('foo', body).subscribe(jasmine.createSpy());
102+
httpMock.expectOne((req) => {
103+
return (
104+
req.method === 'GET' &&
105+
req.urlWithParams === 'foo?formKey=value' &&
106+
!req.body
107+
);
108+
});
109+
});
110+
111+
it('using json', () => {
112+
const body = {key: 'value'};
113+
store.overrideSelector(getIsFeatureFlagsLoaded, true);
114+
store.overrideSelector(getIsInColab, true);
115+
tbHttpClient.post('foo', body).subscribe(jasmine.createSpy());
116+
httpMock.expectOne((req) => {
117+
return (
118+
req.method === 'GET' &&
119+
req.urlWithParams === 'foo?key=value' &&
120+
!req.body
121+
);
122+
});
123+
});
124+
125+
it('sets body as a serialized query param when serializeUnder is set', () => {
126+
const body = {key: 'value', foo: [1, 2, 3]};
127+
store.overrideSelector(getIsFeatureFlagsLoaded, true);
128+
store.overrideSelector(getIsInColab, true);
129+
tbHttpClient
130+
.post('foo', body, {}, 'request')
131+
.subscribe(jasmine.createSpy());
132+
httpMock.expectOne((req) => {
133+
return (
134+
req.method === 'GET' &&
135+
req.urlWithParams ===
136+
'foo?request=%7B%22key%22:%22value%22,%22foo%22:%5B1,2,3%5D%7D' &&
137+
!req.body
138+
);
139+
});
107140
});
108141
});
109142

0 commit comments

Comments
 (0)