-
Notifications
You must be signed in to change notification settings - Fork 332
Implement functionality to open and close projects in dashboard #13994
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
Conversation
586865b
to
cb8fbd0
Compare
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.
My a bit delayed twopenny:
// This is a Vue function, not a React hook, so the React hooks rule doesn't apply | ||
// eslint-disable-next-line react-hooks/rules-of-hooks | ||
const enableProjectService = useFeatureFlag('enableProjectService') | ||
if (!enableProjectService.value) { | ||
this.socketPromise = this.reconnect() | ||
} |
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.
You don't make use of Vue reactivity here, so you may just add getFeatureFlag
method there with body:
return flagsStore.getState().featureFlags[key]
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.
Also, one thing looks strange: we reconnect only when ProjectService is not enabled? I thought it should be the other way round...
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 see, there's some confusion in the naming. The new logic is called ProjectService because there is a ProjectService.scala which I ported to TypeScript. In other words, the project service is the new logic in dashboard and the project manager is the old java process
- name: 📥 Download Enso Engine | ||
continue-on-error: true | ||
working-directory: app/project-manager-shim | ||
run: pnpm run download-engine | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
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 this a check if we're able to download engine, or we need to download engine for one of these checks?
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 need the enso executable to run the ProjectService.test.ts
test suite
// Check if the project is already running | ||
const runningProject = this.runningProjects.get(projectId) | ||
if (runningProject) { | ||
// Return the existing language server sockets | ||
return runningProject.sockets | ||
} | ||
|
||
// Generate a random root ID for this language server session | ||
const rootId = crypto.randomUUID() | ||
|
||
// Find available ports for the language server | ||
const jsonPort = await this.findAvailablePort(DEFAULT_JSONRPC_PORT) |
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.
What if runningProject
changes while we're searching for available port, or stuck at any other await
in this function?
@4e6, please update https://github.com/enso-org/enso/blob/develop/docs/CONTRIBUTING.md#running-ide with the new recommended steps. |
app/gui/src/dashboard/services/ProjectManager/ProjectManager.ts
Outdated
Show resolved
Hide resolved
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.
CR ✅
Pull Request Description
close #13891
close #14006
followup #13832
PR adds the rest of the project management functionality to the dashboard.
Changelog:
project/open
project/close
project/delete
project/duplicate
project/rename
ProjectService.test.ts
test suite checking the project management logicenableProjectService
feature flag is disabledImportant Notes
How to enable the new dashboard project management
Dev Mode
Run the dev server as usual (you can skip the rest if you only develop gui)
Locate enso executable
The
dev:gui
script looks into thebuilt-distribution/
directory for anenso
executable (enso.exe
orenso.bat
on Windows). This way it will automatically pick up the dev enso version built withsbt buildEngineDistribution
if nothing was found, the script will download the latest nightly
Override enso executable
ENSO_RUNNER_PATH
environment variable can be used to set the absolute path to specificenso
executableElectron app
If you want to enable new project management logic in Electron, you have to manually edit the local storage and enable the feature flag:
enableProjectService: true
(run the app with--debug.dev-tools
cli option)Then set the
ENSO_RUNNER_PATH
environment variable and re-run the app (you can add--no-server
flag to ensure no project manager process running)Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.