-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
|
||
export async function start(context: theia.PluginContext) { | ||
const machineToken = process.env['CHE_MACHINE_TOKEN']; | ||
const isMultiUser = !!(machineToken && machineToken.length > 0); | ||
const axiosInstance: AxiosInstance = axios; | ||
const cheApi = process.env['CHE_API']; | ||
const isHostedChe = cheApi && cheApi.indexOf('https://che.openshift.io/api') !== -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we somehow avoid hardcoding of domain name here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make an API request but I think it is not a good solution.
|
||
export async function start(context: theia.PluginContext) { | ||
const machineToken = process.env['CHE_MACHINE_TOKEN']; | ||
const isMultiUser = !!(machineToken && machineToken.length > 0); | ||
const axiosInstance: AxiosInstance = axios; | ||
const cheApi = process.env['CHE_API']; | ||
const isHostedChe = cheApi && cheApi.indexOf('https://che.openshift.io/api') !== -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it looking wired to hardcode an hosted platform there ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, for example there is preview platform as well, etc, so maybe we need another way of saying it's an 'hosted che' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any other ideas how to detect Hosted Che
?
package.json
Outdated
@@ -18,7 +18,8 @@ | |||
"@theia/plugin-packager": "latest", | |||
"rimraf": "2.6.2", | |||
"typescript-formatter": "7.2.2", | |||
"typescript": "2.9.2" | |||
"typescript": "2.9.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason to not use same version of typescript of theia/che-theia ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set the version from che-theia
@vitaliy-guliy @benoitf Do you have any objections to merge it? |
return new Promise<string>(async (resolve, reject) => { | ||
if (isHostedChe) { | ||
const user = await che.user.getCurrentUser(); | ||
const osUserResponse = await axiosInstance.get<OsUserResponse>('https://api.openshift.io/api/users?filter[username]=' + user.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's possible to remove declarations of isHostedChe
and cheApi
and always ask the API.
if (process.env['CHE_API']) {
const user = await che.user.getCurrentUser();
const osUserResponse = await axiosInstance.get<OsUserResponse>(process.env['CHE_API'] + '/users?filter[username]=' + user.name);
resolve(osUserResponse.data.data[0].attributes.cluster);
return;
}
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This request is redundant in case when the plugin doesn't run on Hosted Che
Let's merge this and make improvements for detecting Hosted Che later. |
Check if plugin is running on Hosted Che, if so call specific Hosted Che token request. See: eclipse-che/che#16890 (comment)
fixes eclipse-che/che#16890