Skip to content
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

Update Codespaces configuration #60900

Merged
merged 1 commit into from
Oct 27, 2021
Merged

Update Codespaces configuration #60900

merged 1 commit into from
Oct 27, 2021

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Oct 27, 2021

  • mark onCreateCommand.sh as executable
  • add more regions so users outside of West US can take advantage of prebuilt codespaces
  • use v1-stable version, as recommended by docs
  • use faster machines for dev productivity

With this configuration, I was able to create a Codespaces VM, and the repo was already built for libs+clr -rc Release. Right after creating the Codespace VM, I did:

image

without building the repo.

To try this out yourself, when creating the new codespace, ensure you pick the 8 core option, as that's the machine type I'm pre-building.

- mark onCreateCommand.sh as executable
- add more regions so users outside of West US can take advantage of prebuilt codespaces
- use v1-stable version, as recommended by docs
- use faster machines for dev productivity
@ghost
Copy link

ghost commented Oct 27, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details
  • mark onCreateCommand.sh as executable
  • add more regions so users outside of West US can take advantage of prebuilt codespaces
  • use v1-stable version, as recommended by docs
  • use faster machines for dev productivity

With this configuration, I was able to create a Codespaces VM, and the repo was already built for libs+clr -rc Release. Right after creating the Codespace VM, I did:

image

without building the repo.

Author: eerhardt
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@eerhardt eerhardt requested a review from safern October 27, 2021 02:55
@danmoseley
Copy link
Member

Exciting! (Is the html logger error in the image unrelated)

Let's get some folks trying this out for real work!

regions: WestUs2
sku_name: standardLinux32gb
regions: WestUs2 EastUs WestEurope
sku_name: premiumLinux
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the implications of which regions we select and which sku we use?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which regions we select

Honestly this took the most time to track down. Which GH server your requests go to depends on your physical location. There are 4 available regions - WestUs2 EastUs WestEurope SouthEastAsia. When I (located in the middle of the US) created a new Codespace I wasn't getting a pre-built image. Instead it would create a new container from scratch. There was no indication of why. I needed to contact GH support, and they let me know that my requests were going to EastUs. So I added EastUs to cover the rest of the US, and WestEurope to cover the team members in Europe. We can add SouthEastAsia in the future, if necessary.

which sku we use

See https://github.com/github/codespaces-precache#valid-sku-names. I switched from 4-core to 8-core. This doubles the price of the machines, but also makes them faster. https://docs.github.com/en/billing/managing-billing-for-github-codespaces/about-billing-for-codespaces#codespaces-pricing.

Originally I did this in order to speed up our build - the libs+clr -rc Release build was taking over 30 minutes on the 4-core machines. Maybe now since they are pre-built it might not matter as much. I figured the dev experience would be better on the 8-core machines. We can scale this back to 4-core (now or later), if we want.

I believe when a dev creates their codespace machine, they need to pick the same size as what is listed here in order to get the pre-built. I will test this out today to verify.

Looking around - Rails is using 16-core largePremiumLinux sku. VSCode is using 4-core.

Copy link
Member

@terrajobst terrajobst Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eerhardt that makes sense. Do we know how this impacts billing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know who gets the bill. @danmoseley ?

It will definitely make the bill go up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know who gets the bill (the dotnet org is part of the DNF enterprise account, so they are being billed). What I don't know is how your changes impact billing. Like, did we just make it 4x as expensive?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eerhardt I tried a few difference sizes and settled on 8-core.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note what Eric pointed out above -- if a dev picks up a pre-built, and then just iterates on a small part of the tree and running tests, they may not need such a beefy machine. @vcsjones I'm guessing you were building the whole tree.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vcsjones I'm guessing you were building the whole tree.

Yeah, but I often end up doing a ./build.sh -clean anyway and rebuilding things from scratch. I probably don't need to as often as I do, but, it's usually the quickest way for me to get unstuck when I have angered all of the tools. Usually I end up doing this as a result of a branch switch (I don't use a codespace per-branch).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@terrajobst the cost is doubled per hour:

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe when a dev creates their codespace machine, they need to pick the same size as what is listed here in order to get the pre-built. I will test this out today to verify.

I tried this. The developer does in fact need to pick the same size machine as we pick here. I created a new 4-core Codespace, and it didn't use a pre-built. I created a new 8-core Codespace, and it did use a pre-built. I will give this feedback to the Codespaces team. It doesn't seem ideal that all the devs need to pick the same size machine.

@eerhardt
Copy link
Member Author

Is the html logger error in the image unrelated

I haven't attempted to trace that down yet. I will try it on other tests to see if the same error shows up. But it happened twice on the Regex tests.

@vcsjones
Copy link
Member

vcsjones commented Oct 27, 2021

Extremely excited for this! Let me know if I can help y'all in any way.

Let's get some folks trying this out for real work!

You can't tell but I've been using Codespaces for nearly every PR I've submitted here for the past 6 months. I will very quickly switch to the "official" support here :-).

Copy link
Member

@fanyang-mono fanyang-mono left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am new to CodeSpace. I don't know where I could go to see the exact build commands for mono?

with:
regions: WestUs2
sku_name: standardLinux32gb
regions: WestUs2 EastUs WestEurope
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know which countries WestEurope include. One of the mono developers lives in Romania. Don't know that counts as WestEurope or EastEurope. What about Czech Republic (Prague)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where I could go to see the exact build commands for mono?

Here's the .sh script that runs during the pre-build process:

./build.sh libs+clr -rc Release

You can edit this script to build whatever we need.

I don't know which countries WestEurope include. One of the mono developers lives in Romania. Don't know that counts as WestEurope or EastEurope. What about Czech Republic (Prague)?

See #60900 (comment) for the available regions.

@danmoseley
Copy link
Member

cc @captainsafia I can't remember whether aspnetcore is already doing pre-built, or you're observing this experiment. Looks like it works.

@eerhardt
Copy link
Member Author

I'm going to merge this to make forward progress.

Failures are #60933 #60934

@eerhardt eerhardt merged commit 8f8e8f0 into main Oct 27, 2021
@eerhardt eerhardt deleted the eerhardt/FixCodeSpaces branch October 27, 2021 19:45
@eerhardt
Copy link
Member Author

eerhardt commented Oct 28, 2021

Is the html logger error in the image unrelated

I looked into it some today. I can't repro it in a standalone test project, but what I think is happening is that the HtmlLogger is trying to serialize an object that contains the character \uFFFE and that is causing an exception here:

https://github.com/microsoft/vstest/blob/ad32654bc155a059fda263f0ec99b445058d84ea/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs#L323-L337

I noticed I'm not getting an .html file written in these tests.

This only happens for Regex tests. I tried a few other libraries, and they were all fine. I may end up logging a bug in microsoft/vstest if I can repro this in a standalone test.

@stephentoub - you run the Regex tests a lot, have you seen this locally?

@danmoseley
Copy link
Member

A logger of errors from unit tests needs to be able to successfully log total garbage, or at least recover gracefully 😄

In a quick look through I don't see any protection in the HtmlLogger against garbage, and no tests verifying garbage is accepted by its event handlers:
https://github.com/microsoft/vstest/blob/ad32654bc155a059fda263f0ec99b445058d84ea/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs

@eerhardt
Copy link
Member Author

I get the same error on windows, so it has nothing to do with Codespaces, or Linux:

image

@eerhardt
Copy link
Member Author

Found it: microsoft/vstest#3136

@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants