feat (codespaces) - Add initial support for codespaces #27483

ShreyasJejurkar · 2022-02-21T07:52:18Z

Added support for codespaces using .NET and SQL Server docker image.
Entire repository can be built and can run tests in the cloud.
Added commonly used tasks to tasks.json can be invoked using pressing CTRL + SHIFT + P and selecting Run Tasks as an option.
Added sql-cmd to the PATH, so that users can access DB right off after the creating container. Developers can access both sql-cmd on the command line and with GUI inside VSCode for SQL Server.
I have built the entire repository successfully and is workable. The inner loop just works nicely!

Once all changes as done as part of PR feedback, planning to include a section for VSCode and codespaces users on how to build, developed, and test EF in the cloud in this doc => https://github.com/dotnet/efcore/blob/main/docs/getting-and-building-the-code.md.

.devcontainer/devcontainer.json

source the path so that user can access `sql-cmd` right off.

ajcvickers · 2022-03-02T09:01:31Z

@captainsafia Kevin mentioned that you had some experience of enabling codespaces. Could you take a look at this and see what you think?

captainsafia · 2022-03-04T00:34:05Z

.devcontainer/devcontainer.json

+ "omnisharp.enableRoslynAnalyzers": true,
+ "omnisharp.enableEditorConfigSupport": true,
+ "omnisharp.enableImportCompletion": true,
+ "omnisharp.useModernNet": true,


Oooh! Interesting. I think we probably want to enable this in the aspnetcore repo too.

You mean the last option right? useModernNet? Yes, right now am using it in codespaces for the aspnetcore repo. But somehow the intellisense got slowed because of this. Am still figuring out that one for the aspnetcore repo. But this works great here in efcore repo.

captainsafia · 2022-03-04T00:36:12Z

.devcontainer/devcontainer.json

+ "settings": {
+ "mssql.connections": [
+ {
+ "server": "localhost,1433",


I assume this is here so the SQL extension can automatically connect to the dev database from VS Code?

Yes, interestingly, I can able to run tests from the efcore repo directly against this DB!

Test__SqlServer__DefaultConnection below makes it happen.

Yes, exactly I did that based on docs on repo!

captainsafia

The Codespaces specific stuff looks legit.

I'm not experienced with the mssql bit of things but it seems like a sensible dev setup.

ShreyasJejurkar · 2022-03-04T05:34:12Z

The Codespaces specific stuff looks legit.

I'm not experienced with the mssql bit of things but it seems like a sensible dev setup.

Yes, I want people from the EF core repo (who do commit every day to this repo) to try this and provide feedback. Because the experience is quite great for old and newcomers as well. We can write code, write tests, execute against local DB which is inside codespace and much more! Please give it a try EF core people!

roji

The SQL Server setup looks good to me, thanks!

I'm no VS Code/Codespaces expert and didn't verify this all the way (a VS Code test runner would need to be installed etc.), but is looks good enough to merge. Will leave this open for a few more days in case someone else from the team feels like playing around with it.

roji · 2022-03-09T15:18:39Z

.devcontainer/mssql/postCreateCommand.sh

+dacpath=$2
+sqlpath=$3
+
+echo "SELECT * FROM SYS.DATABASES" | dd of=testsqlconnection.sql


Any reason not to simplify to:

Suggested change

echo "SELECT * FROM SYS.DATABASES" | dd of=testsqlconnection.sql

echo "SELECT * FROM SYS.DATABASES" > testsqlconnection.sql

This script was automatically created when we asked codespaces to include support for SQL Server. So I didn't touch.
Sorry for the late reply! :(

roji · 2022-03-09T15:19:53Z

.devcontainer/mssql/postCreateCommand.sh

+echo "SELECT * FROM SYS.DATABASES" | dd of=testsqlconnection.sql
+for i in {1..60};
+do
+ /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P $SApassword -d master -i testsqlconnection.sql > /dev/null


Or even better: sqlcmd supports providing a query on the cmdline with -Q, can use that instead of a temp file

the script was created by codespaces itself. I didn't write this!

If we can improve it by any means, I am ok with it! :)

ajcvickers · 2022-03-10T15:22:24Z

Thanks @ShreyasJejurkar for the PR, and thanks @captainsafia for the review.

smitpatel · 2022-03-15T22:40:52Z

.devcontainer/devcontainer.json

+ "DOTNET_MULTILEVEL_LOOKUP": "0",
+ "TARGET": "net7.0",
+ "DOTNET_WATCH_SUPPRESS_LAUNCH_BROWSER": "true",
+ "Test__SqlServer__DefaultConnection ": "Server=localhost;Database=master;User Id=sa;password=Pass@word;Trusted_Connection=False;MultipleActiveResultSets=true;"


Is space at the end of env var correct? While repo will build/test fine if SqlServer is not running, need to see if SqlServer tests actually ran.

Yes, before creating this PR, I run all tests with the script which is at the root of the repo, not even a single test case failed!

This reverts commit 9599197.

captainsafia · 2022-03-17T16:20:44Z

@ajcvickers Aw shucks! Any reason this needed to be reverted? I'm trying to stay aware of any Codespace-related issue affecting our repos.

ajcvickers · 2022-03-17T16:28:19Z

@captainsafia "Passwords are not allowed in config files." Will forward you the internal email on this.

…" (#27655)

ShreyasJejurkar added 3 commits February 21, 2022 07:49

feat (codespaces) - Add codespaces support.

4b3eb8e

Add commonly used tasks to tasks.json

cbe6d99

Add connection string env variable for running tests

8b4e731

ShreyasJejurkar commented Feb 21, 2022

View reviewed changes

.devcontainer/devcontainer.json Outdated Show resolved Hide resolved

ShreyasJejurkar added 2 commits February 21, 2022 08:15

source sqlcmd path while creating container

e23e916

source the path so that user can access `sql-cmd` right off.

testing connection string for tests

4d12507

captainsafia reviewed Mar 4, 2022

View reviewed changes

captainsafia approved these changes Mar 4, 2022

View reviewed changes

ajcvickers assigned roji Mar 4, 2022

roji approved these changes Mar 9, 2022

View reviewed changes

ajcvickers merged commit 9599197 into dotnet:main Mar 10, 2022

ajcvickers added the community-contribution label Mar 10, 2022

smitpatel reviewed Mar 15, 2022

View reviewed changes

ajcvickers added a commit that referenced this pull request Mar 16, 2022

Revert "feat (codespaces) - Add initial support for codespaces (#27483)"

896cd64

This reverts commit 9599197.

ajcvickers mentioned this pull request Mar 17, 2022

.NET Data Biweekly Updates (2022) #27185

Closed

ajcvickers added a commit that referenced this pull request Mar 18, 2022

Revert "feat (codespaces) - Add initial support for codespaces (#27483)…

e0a4b88

…" (#27655)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat (codespaces) - Add initial support for codespaces #27483

feat (codespaces) - Add initial support for codespaces #27483

ShreyasJejurkar commented Feb 21, 2022 •

edited

Loading

ajcvickers commented Mar 2, 2022

captainsafia Mar 4, 2022

ShreyasJejurkar Mar 4, 2022

captainsafia Mar 4, 2022

ShreyasJejurkar Mar 4, 2022

smitpatel Mar 15, 2022

ShreyasJejurkar Mar 17, 2022

captainsafia left a comment

ShreyasJejurkar commented Mar 4, 2022 •

edited

Loading

roji left a comment

roji Mar 9, 2022

ShreyasJejurkar Mar 17, 2022

roji Mar 9, 2022

ShreyasJejurkar Mar 17, 2022

ajcvickers commented Mar 10, 2022

smitpatel Mar 15, 2022

ShreyasJejurkar Mar 17, 2022

captainsafia commented Mar 17, 2022

ajcvickers commented Mar 17, 2022

	echo "SELECT * FROM SYS.DATABASES" \| dd of=testsqlconnection.sql
	echo "SELECT * FROM SYS.DATABASES" > testsqlconnection.sql

feat (codespaces) - Add initial support for codespaces #27483

feat (codespaces) - Add initial support for codespaces #27483

Conversation

ShreyasJejurkar commented Feb 21, 2022 • edited Loading

ajcvickers commented Mar 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainsafia left a comment

Choose a reason for hiding this comment

ShreyasJejurkar commented Mar 4, 2022 • edited Loading

roji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajcvickers commented Mar 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainsafia commented Mar 17, 2022

ajcvickers commented Mar 17, 2022

ShreyasJejurkar commented Feb 21, 2022 •

edited

Loading

ShreyasJejurkar commented Mar 4, 2022 •

edited

Loading