Skip to content

Commit a12bab7

Browse files
fcollonvalfcollonval
authored
Simplify request API interface (#764)
* New request API interface * Fix status widget and GitResponseError
1 parent 2d8d659 commit a12bab7

18 files changed

+1164
-1457
lines changed

‎.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ repos:
33
rev: 20.8b1 # Replace by any tag/version: https://github.com/psf/black/tags
44
hooks:
55
- id: black
6-
language_version: python3 # Should be a command that runs python3.6+
6+
language_version: python3 # Should be a command that runs python3.6+

‎src/components/GitPanel.tsx

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -575,33 +575,35 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
575575
// If the repository path changes, check the user identity
576576
if (path !== this._previousRepoPath) {
577577
try {
578-
let res = await this.props.model.config();
579-
if (res.ok) {
580-
const options: JSONObject = (await res.json()).options;
581-
const keys = Object.keys(options);
582-
583-
// If the user name or e-mail is unknown, ask the user to set it
584-
if (keys.indexOf('user.name') < 0 || keys.indexOf('user.email') < 0) {
585-
const result = await showDialog({
586-
title: 'Who is committing?',
587-
body: new GitAuthorForm()
588-
});
589-
if (!result.button.accept) {
590-
console.log('User refuses to set identity.');
591-
return false;
592-
}
593-
const identity = result.value;
594-
res = await this.props.model.config({
578+
const data: JSONObject = (await this.props.model.config()) as any;
579+
const options: JSONObject = data['options'] as JSONObject;
580+
const keys = Object.keys(options);
581+
582+
// If the user name or e-mail is unknown, ask the user to set it
583+
if (keys.indexOf('user.name') < 0 || keys.indexOf('user.email') < 0) {
584+
const result = await showDialog({
585+
title: 'Who is committing?',
586+
body: new GitAuthorForm()
587+
});
588+
if (!result.button.accept) {
589+
console.log('User refuses to set identity.');
590+
return false;
591+
}
592+
const identity = result.value;
593+
try {
594+
await this.props.model.config({
595595
'user.name': identity.name,
596596
'user.email': identity.email
597597
});
598-
if (!res.ok) {
599-
console.log(await res.text());
598+
} catch (error) {
599+
if (error instanceof Git.GitResponseError) {
600+
console.log(error);
600601
return false;
601602
}
603+
throw error;
602604
}
603-
this._previousRepoPath = path;
604605
}
606+
this._previousRepoPath = path;
605607
} catch (error) {
606608
throw new Error('Failed to set your identity. ' + error.message);
607609
}

‎src/components/diff/NbDiff.tsx

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { PathExt } from '@jupyterlab/coreutils';
22
import * as nbformat from '@jupyterlab/nbformat';
33
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
4-
import { ServerConnection } from '@jupyterlab/services/lib/serverconnection';
54
import { IDiffEntry } from 'nbdime/lib/diff/diffentries';
65
import { CellDiffModel, NotebookDiffModel } from 'nbdime/lib/diff/model';
76
import { CellDiffWidget } from 'nbdime/lib/diff/widget';
87
import * as React from 'react';
98
import { RefObject } from 'react';
10-
import { httpGitRequest } from '../../git';
9+
import { requestAPI } from '../../git';
10+
import { IDiffProps, RenderMimeConsumer } from './Diff';
1111
import { IDiffContext } from './model';
1212
import { NBDiffHeader } from './NBDiffHeader';
13-
import { IDiffProps, RenderMimeConsumer } from './Diff';
1413

1514
export interface ICellDiffProps {
1615
cellChunk: CellDiffModel[];
@@ -167,53 +166,42 @@ export class NBDiff extends React.Component<IDiffProps, INBDiffState> {
167166
* @param diffContext the context in which to perform the diff
168167
*/
169168
private performDiff(diffContext: IDiffContext): void {
170-
try {
171-
// Resolve what API parameter to call.
172-
let currentRefValue: any;
173-
if ('specialRef' in diffContext.currentRef) {
174-
currentRefValue = {
175-
special: diffContext.currentRef.specialRef
176-
};
177-
} else {
178-
currentRefValue = {
179-
git: diffContext.currentRef.gitRef
180-
};
181-
}
169+
// Resolve what API parameter to call.
170+
let currentRefValue: any;
171+
if ('specialRef' in diffContext.currentRef) {
172+
currentRefValue = {
173+
special: diffContext.currentRef.specialRef
174+
};
175+
} else {
176+
currentRefValue = {
177+
git: diffContext.currentRef.gitRef
178+
};
179+
}
182180

183-
httpGitRequest('/nbdime/api/gitdiff', 'POST', {
181+
requestAPI<INbdimeDiff>(
182+
'gitdiff',
183+
'POST',
184+
{
184185
file_path: PathExt.join(this.props.topRepoPath, this.props.path),
185186
ref_local: { git: diffContext.previousRef.gitRef },
186187
ref_remote: currentRefValue
187-
}).then((response: Response) => {
188-
response
189-
.json()
190-
.then((data: INbdimeDiff) => {
191-
if (response.status !== 200) {
192-
// Handle error
193-
this.setState({
194-
errorMessage:
195-
data.message || 'Unknown error. Please check the server log.'
196-
});
197-
} else {
198-
// Handle response
199-
const base = data.base;
200-
const diff = data.diff;
201-
const nbdModel = new NotebookDiffModel(base, diff);
202-
this.setState({
203-
nbdModel: nbdModel
204-
});
205-
}
206-
})
207-
.catch(reason => {
208-
// Handle error
209-
this.setState({
210-
errorMessage:
211-
reason.message || 'Unknown error. Please check the server log.'
212-
});
213-
});
188+
},
189+
'nbdime/api'
190+
)
191+
.then((data: INbdimeDiff) => {
192+
const base = data.base;
193+
const diff = data.diff;
194+
const nbdModel = new NotebookDiffModel(base, diff);
195+
this.setState({
196+
nbdModel: nbdModel
197+
});
198+
})
199+
.catch(reason => {
200+
// Handle error
201+
this.setState({
202+
errorMessage:
203+
reason.message || 'Unknown error. Please check the server log.'
204+
});
214205
});
215-
} catch (err) {
216-
throw ServerConnection.NetworkError;
217-
}
218206
}
219207
}

‎src/components/diff/PlainTextDiff.tsx

Lines changed: 30 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import { Mode } from '@jupyterlab/codemirror';
2-
import { ServerConnection } from '@jupyterlab/services';
3-
42
import * as React from 'react';
5-
3+
import { requestAPI } from '../../git';
4+
import { Git } from '../../tokens';
65
import { IDiffProps } from './Diff';
7-
import { httpGitRequest } from '../../git';
86
import { mergeView } from './mergeview';
97
import { IDiffContext } from './model';
108

@@ -20,7 +18,7 @@ export interface IPlainTextDiffState {
2018
/**
2119
* A React component to render the diff of a plain text file
2220
*
23-
* 1. It calls the `/git/diffcontent` API on the server to get the previous and current content
21+
* 1. It calls the `diffcontent` API on the server to get the previous and current content
2422
* 2. Renders the content using CodeMirror merge addon
2523
*/
2624
export class PlainTextDiff extends React.Component<
@@ -66,51 +64,35 @@ export class PlainTextDiff extends React.Component<
6664
* @param diffContext the context in which to perform the diff
6765
*/
6866
private _performDiff(diffContext: IDiffContext): void {
69-
try {
70-
// Resolve what API parameter to call.
71-
let currentRefValue: ICurrentReference;
72-
if ('specialRef' in diffContext.currentRef) {
73-
currentRefValue = {
74-
special: diffContext.currentRef.specialRef
75-
};
76-
} else {
77-
currentRefValue = {
78-
git: diffContext.currentRef.gitRef
79-
};
80-
}
67+
// Resolve what API parameter to call.
68+
let currentRefValue: ICurrentReference;
69+
if ('specialRef' in diffContext.currentRef) {
70+
currentRefValue = {
71+
special: diffContext.currentRef.specialRef
72+
};
73+
} else {
74+
currentRefValue = {
75+
git: diffContext.currentRef.gitRef
76+
};
77+
}
8178

82-
httpGitRequest('/git/diffcontent', 'POST', {
83-
filename: this.props.path,
84-
prev_ref: { git: diffContext.previousRef.gitRef },
85-
curr_ref: currentRefValue,
86-
top_repo_path: this.props.topRepoPath
87-
}).then(response => {
88-
response
89-
.json()
90-
.then(data => {
91-
if (response.status !== 200) {
92-
// Handle error
93-
this.setState({
94-
errorMessage:
95-
data.message || 'Unknown error. Please check the server log.'
96-
});
97-
} else {
98-
this._addDiffViewer(data['prev_content'], data['curr_content']);
99-
}
100-
})
101-
.catch(reason => {
102-
console.error(reason);
103-
// Handle error
104-
this.setState({
105-
errorMessage:
106-
reason.message || 'Unknown error. Please check the server log.'
107-
});
108-
});
79+
requestAPI<Git.IDiffContent>('diffcontent', 'POST', {
80+
filename: this.props.path,
81+
prev_ref: { git: diffContext.previousRef.gitRef },
82+
curr_ref: currentRefValue as any,
83+
top_repo_path: this.props.topRepoPath
84+
})
85+
.then(data => {
86+
this._addDiffViewer(data['prev_content'], data['curr_content']);
87+
})
88+
.catch(reason => {
89+
console.error(reason);
90+
// Handle error
91+
this.setState({
92+
errorMessage:
93+
reason.message || 'Unknown error. Please check the server log.'
94+
});
10995
});
110-
} catch (err) {
111-
console.error(err);
112-
throw ServerConnection.NetworkError;
113-
}
11496
}
11597

11698
/**

‎src/git.ts

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { URLExt } from '@jupyterlab/coreutils';
22
import { ServerConnection } from '@jupyterlab/services';
3+
import { ReadonlyJSONObject } from '@lumino/coreutils';
4+
import { Git } from './tokens';
35

46
/**
57
* Array of Git Auth Error Messages
@@ -10,25 +12,69 @@ export const AUTH_ERROR_MESSAGES = [
1012
'could not read Password'
1113
];
1214

13-
/** Makes a HTTP request, sending a git command to the backend */
14-
export function httpGitRequest(
15-
url: string,
16-
method: string,
17-
request: Record<string, any> | null
18-
): Promise<Response> {
19-
let fullRequest: RequestInit;
20-
if (request === null) {
21-
fullRequest = {
22-
method: method
23-
};
24-
} else {
25-
fullRequest = {
26-
method: method,
27-
body: JSON.stringify(request)
28-
};
15+
/**
16+
* Call the API extension
17+
*
18+
* @param endPoint API REST end point for the extension; default ''
19+
* @param method HTML method; default 'GET'
20+
* @param body JSON object to be passed as body or null; default null
21+
* @param namespace API namespace; default 'git'
22+
* @returns The response body interpreted as JSON
23+
*
24+
* @throws {Git.GitResponseError} If the server response is not ok
25+
* @throws {ServerConnection.NetworkError} If the request cannot be made
26+
*/
27+
export async function requestAPI<T>(
28+
endPoint = '',
29+
method = 'GET',
30+
body: ReadonlyJSONObject | null = null,
31+
namespace = 'git'
32+
): Promise<T> {
33+
// Make request to Jupyter API
34+
const settings = ServerConnection.makeSettings();
35+
const requestUrl = URLExt.join(
36+
settings.baseUrl,
37+
namespace, // API Namespace
38+
endPoint
39+
);
40+
41+
const init: RequestInit = {
42+
method,
43+
body: body ? JSON.stringify(body) : undefined
44+
};
45+
46+
let response: Response;
47+
try {
48+
response = await ServerConnection.makeRequest(requestUrl, init, settings);
49+
} catch (error) {
50+
throw new ServerConnection.NetworkError(error);
51+
}
52+
53+
let data: any = await response.text();
54+
let isJSON = false;
55+
if (data.length > 0) {
56+
try {
57+
data = JSON.parse(data);
58+
isJSON = true;
59+
} catch (error) {
60+
console.log('Not a JSON response body.', response);
61+
}
62+
}
63+
64+
if (!response.ok) {
65+
if (isJSON) {
66+
const { message, traceback, ...json } = data;
67+
throw new Git.GitResponseError(
68+
response,
69+
message ||
70+
`Invalid response: ${response.status} ${response.statusText}`,
71+
traceback || '',
72+
json
73+
);
74+
} else {
75+
throw new Git.GitResponseError(response, data);
76+
}
2977
}
3078

31-
const setting = ServerConnection.makeSettings();
32-
const fullUrl = URLExt.join(setting.baseUrl, url);
33-
return ServerConnection.makeRequest(fullUrl, fullRequest, setting);
79+
return data;
3480
}

0 commit comments

Comments
 (0)