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

Removed internalization of one-time-use pkgs #28

Merged
merged 2 commits into from
Jul 20, 2021
Merged

Removed internalization of one-time-use pkgs #28

merged 2 commits into from
Jul 20, 2021

Conversation

adilio
Copy link
Contributor

@adilio adilio commented Jul 20, 2021

Closes #19 .

ThisPR removes code that internalizes one-time-use packages, as they won't be used again and slow down initial setup. This will greatly improve speed of total setup.

Copy link
Contributor Author

@adilio adilio left a comment

Choose a reason for hiding this comment

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

need typo fix

Start-C4BSetup.ps1 Outdated Show resolved Hide resolved
Copy link
Member

@ryanrichter94 ryanrichter94 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@adilio adilio merged commit 12b62b3 into chocolatey:main Jul 20, 2021
@adilio adilio deleted the 19-dont-internalize-pkgs branch July 20, 2021 20:21
@steviecoaster
Copy link
Contributor

I'm gonna request we pull this back out. Sure it speeds up initial setup, but what happens if they need to reinstall something? Gonna need those dependencies in the repo.

@ryanrichter94
Copy link
Member

Upon further review and reconsideration. I'm with Stephen that we internalize the CCM packages. We often recommend internalizing the packages from the licensed feed on support calls. It's only right we set good first practice by internalizing these packages and implementing what we preach.

As a side note to this i'd like to play around with creating a Jenkins job that we ship with this, and possibly any future QDE releases we do, that would internalize the packages from the chocolatey.licensed source, push them to the test repo. And have the job trigger the already existing Update Production Repository Job.

@adilio
Copy link
Contributor Author

adilio commented Jul 20, 2021

So I've just gone over this with @steviecoaster.

I've tested this on Windows Server 2016, 2019, and 2022 multiple times. Leaving the internalization and push to Nexus in the setup adds at least 20 minutes to the install time. If their box is any slower or their WAN connection slow in any way, this could increase the time by 30 minutes to an hour.

Let's break down what is actually changing. We are leaving out the following packages:

  • sql-server-express
  • sql-server-management-studio
  • dotnet4.6.1

These have dependency on large KB packages, one of which is 2.7GB. These KB's also no-op anyway, since they're only relevant to 2012 R2. I see the above as one-time-use only packages, and the benefit of internalizing them only to push them into a repo and then use them immediately after is of very limited utility. If they're somehow unavailable at install, then it's just as likely they won't be available at internalize.

The other packages we're leaving out are Chocolatey's own packages from the licensed feed. These aren't that large, and I'm not opposed to putting them back in based on @ryanrichter94's rationale above. I'll create a separate issue for this, and submit a separate PR.

@steviecoaster & @ryanrichter94 , I've discussed it with you both individually, and we've agreed to leave this PR merged for the time being. We can always revisit later.

Hope that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't internalize and push one-time use packages to Nexus (speed enhancement)
3 participants