This repository has been archived by the owner on Apr 4, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 111
fix(workpace-plugin): Importing several private projects in a single devfile #959
Merged
sunix
merged 5 commits into
eclipse-che:master
from
sunix:fix-workspace-plugin-import-multiple-private-projects
Jan 12, 2021
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
5c7575a
fix(workpace-plugin): Importing several private projects in a single …
sunix 99e7489
refactored, simplify sequence execution with Mutex
sunix 13c0a96
revert imports jest
sunix 004edb9
adding back mutex in lock file
sunix 7e31b9f
removing import jest
sunix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
apiVersion: 1.0.0 | ||
|
||
metadata: | ||
generateName: che-theia-workspace-plugin- | ||
|
||
projects: | ||
- name: che-theia | ||
source: | ||
location: 'https://github.com/eclipse/che-theia.git' | ||
type: git | ||
|
||
components: | ||
- mountSources: true | ||
command: | ||
- tail | ||
- '-f' | ||
- /dev/null | ||
memoryLimit: 512Gi | ||
type: dockerimage | ||
image: 'quay.io/eclipse/che-theia:next' | ||
alias: che-theia-next-dev | ||
env: | ||
- value: 0.0.0.0 | ||
name: THEIA_HOST | ||
- value: '3130' | ||
name: THEIA_PORT | ||
- value: '0' | ||
name: NODE_TLS_REJECT_UNAUTHORIZED | ||
|
||
- mountSources: true | ||
memoryLimit: 3Gi | ||
type: dockerimage | ||
image: 'quay.io/eclipse/che-theia-dev:next' | ||
alias: che-dev | ||
|
||
- id: redhat/vscode-yaml/latest | ||
type: chePlugin | ||
|
||
- id: che-incubator/typescript/latest | ||
memoryLimit: 2048M | ||
type: chePlugin | ||
|
||
- id: ms-vscode/vscode-github-pullrequest/latest | ||
type: chePlugin | ||
|
||
commands: | ||
|
||
- name: build ... workspace-plugin | ||
actions: | ||
- workdir: /projects/che-theia/plugins/workspace-plugin | ||
type: exec | ||
command: | | ||
killall node; yarn || (yarn lint:fix && yarn format:fix && yarn) && echo -e "\e[32mDone.\e[0m build ... workspace-plugin" | ||
component: che-dev | ||
|
||
- name: test-watch ... workspace-plugin | ||
actions: | ||
- workdir: /projects/che-theia/plugins/workspace-plugin | ||
type: exec | ||
command: | | ||
killall node; yarn test:watch | ||
component: che-dev | ||
|
||
- name: run ... che-theia + workspace-plugin | ||
previewUrl: | ||
port: 3130 | ||
actions: | ||
- workdir: /home/theia | ||
type: exec | ||
command: | | ||
rm /default-theia-plugins/eclipse_che_workspace_plugin.theia; mkdir -p /tmp/theiadev_projects && export CHE_PROJECTS_ROOT=/tmp/theiadev_projects && cp /projects/che-theia/plugins/workspace-plugin/eclipse_che_workspace_plugin.theia /default-theia-plugins/ && /entrypoint.sh | ||
component: che-theia-next-dev |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
"src" | ||
], | ||
"dependencies": { | ||
"async-mutex": "^0.2.6", | ||
"fs-extra": "7.0.1" | ||
}, | ||
"devDependencies": { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/********************************************************************** | ||
* Copyright (c) 2019-2021 Red Hat, Inc. | ||
* | ||
* This program and the accompanying materials are made | ||
* available under the terms of the Eclipse Public License 2.0 | ||
* which is available at https://www.eclipse.org/legal/epl-2.0/ | ||
* | ||
* SPDX-License-Identifier: EPL-2.0 | ||
***********************************************************************/ | ||
import { Mutex } from 'async-mutex'; | ||
|
||
export class PromptManager { | ||
private askPassResults: Map<Symbol, string> = new Map(); | ||
private mutex: Mutex = new Mutex(); | ||
|
||
constructor(promptLauncher: (host: string, placeHolder: string) => PromiseLike<string | undefined>) { | ||
this.askPassPromptLauncher = promptLauncher; | ||
} | ||
|
||
async askPass(host: string, placeHolder: string): Promise<string> { | ||
const release = await this.mutex.acquire(); | ||
try { | ||
const key = getKey(host, placeHolder); | ||
if (this.askPassResults.has(key)) { | ||
return this.askPassResults.get(key) || ''; | ||
} | ||
const result = (await this.askPassPromptLauncher(host, placeHolder)) || ''; | ||
this.askPassResults.set(key, result); | ||
return result; | ||
} finally { | ||
release(); | ||
} | ||
} | ||
|
||
askPassPromptLauncher: (host: string, placeHolder: string) => PromiseLike<string | undefined>; | ||
} | ||
|
||
function getKey(host: string, placeHolder: string) { | ||
return Symbol.for(`key[${host}:${placeHolder}]`); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
52 changes: 52 additions & 0 deletions
52
plugins/workspace-plugin/tests/askpass-prompt-manager.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/********************************************************************** | ||
* Copyright (c) 2019-2021 Red Hat, Inc. | ||
* | ||
* This program and the accompanying materials are made | ||
* available under the terms of the Eclipse Public License 2.0 | ||
* which is available at https://www.eclipse.org/legal/epl-2.0/ | ||
* | ||
* SPDX-License-Identifier: EPL-2.0 | ||
***********************************************************************/ | ||
|
||
import { PromptManager } from '../src/askpass-prompt-manager'; | ||
|
||
describe('testing async prompt request', () => { | ||
let iteration: number; | ||
let promptManager: PromptManager; | ||
|
||
beforeEach(() => { | ||
iteration = 0; | ||
promptManager = new PromptManager( | ||
(host: string, placeHolder: string) => | ||
new Promise(resolve => { | ||
iteration += 1; | ||
setTimeout(resolve, Math.floor(Math.random() * 500) + 1000); | ||
console.log(`resolving ${host} ${placeHolder} iteration: ${iteration}`); | ||
resolve(`hello ${host} ${placeHolder} - iteration: ${iteration}`); | ||
}) | ||
); | ||
}); | ||
|
||
test('the prompt promise should be executed in sequence in the call order', async () => { | ||
const askpass1promise = promptManager.askPass('host', 'username'); | ||
const askpass2promise = promptManager.askPass('host', 'password'); | ||
expect(await askpass2promise).toBe('hello host password - iteration: 2'); | ||
expect(await askpass1promise).toBe('hello host username - iteration: 1'); | ||
}); | ||
|
||
test('a prompt with the same host and placeHolder should return the same value and not be executed twice', async () => { | ||
const askpass1promise = promptManager.askPass('host', 'username'); | ||
const askpass2promise = promptManager.askPass('host', 'password'); | ||
const askpass1promise_bis = promptManager.askPass('host', 'username'); | ||
const askpass3promise = promptManager.askPass('host2', 'username'); | ||
const askpass4promise = promptManager.askPass('host2', 'password'); | ||
const askpass3promise_bis = promptManager.askPass('host2', 'username'); | ||
|
||
expect(await askpass2promise).toBe('hello host password - iteration: 2'); | ||
expect(await askpass3promise).toBe('hello host2 username - iteration: 3'); | ||
expect(await askpass1promise).toBe('hello host username - iteration: 1'); | ||
expect(await askpass1promise_bis).toBe('hello host username - iteration: 1'); | ||
expect(await askpass4promise).toBe('hello host2 password - iteration: 4'); | ||
expect(await askpass3promise_bis).toBe('hello host2 username - iteration: 3'); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm not sure it's a good practice to introduce devfiles in each directory ?
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 a devfile to work on the workspace plugin, as this project lives in a subdirectory of the git repo, it looks good to have it there, next to the package.json, etc ...
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.
but it's part of a yarn workspace so you should use top-level folder/definitions/devfile anyway to inherits from all workspace definition
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.
ok but isn't it overkill when a dev just want to work on 1 plugin ?
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.
It might be useful to have the devfile here, even just for reference purposes. It could be useful to point for new contributors in the future.
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 devfile is for coding the workspace plugin without having to rebuild che-theia and all the other plugins. It doesn't make sense to have it elsewhere.
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.
The maintenance issue is not related to the location of the devfiles. but how a devfile could be extended: so the devfile for the workspace plugin (that would be in the workspace plugin folder) could extend a more generic devfile for built-in plugins but adding its own specifics (for instance, custom commands, etc ...). Maybe it is in the scope of devfile 2.
Parent pom.xml files are usually there to share BoM. But without these sharing capabilities, we would have pom.xml files for each module that would be also hard to maintain. Anyway a module would have its own pom.xml.
Anyway, the devfile for the workspace plugin has to stay here because it would setup the right workspace for the workspace-plugin developer flow.
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.
and then if the suggestion is to move that devfile to the
devfiles
directory I don't see how easier it would be to maintain. Or is the suggestion not to have devfile at all and let all the developers loosing time to setup an dev environment to work on each individual plugins?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.
No, the suggestion is to have a a single devfile for developing 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.
there is that devfile https://github.com/eclipse/che-theia/blob/master/devfiles/che-theia-all.devfile.yaml but it is doing way too many things when it is to work a a particular plugin.