Skip to content

flip from start-bits to webclient. remove unused param - resolves #119 - #120

Merged
Josh Wittner (jwittner) merged 4 commits into
microsoft:developfrom
nickhorvath:nick/change_startbits
Sep 4, 2018
Merged

flip from start-bits to webclient. remove unused param - resolves #119#120
Josh Wittner (jwittner) merged 4 commits into
microsoft:developfrom
nickhorvath:nick/change_startbits

Conversation

@nickhorvath

@nickhorvath Nick Horvath (nickhorvath) commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

start-bits doesn't work on VSTS build agents. Webclient calls do.

@jwittner

Josh Wittner (jwittner) commented Aug 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Nick Horvath (@nickhorvath) Hey! Just tried this locally:

$installers = Find-UnitySetupInstaller -Version 2018.2.3f1 -Components Windows, Linux, Mac, UWP, UWP_IL2CPP, Android, iOS, Vuforia
Install-UnitySetupInstance -Installers $installers

And I get this error:

Cannot convert argument "address", with value: "System.Object[]", for "DownloadFile" to type "System.Uri": "Cannot
convert the "System.Object[]" value of type "System.Object[]" to type "System.Uri"."
At C:\Users\jowitt\Source\UnitySetup.Powershell\UnitySetup\UnitySetup.psm1:470 char:13
+             (New-Object System.Net.WebClient).DownloadFile($downloadS ...
+             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodArgumentConversionInvalidCastArgument

See my comment in the code for what I believe the issue is. Thanks a ton for the contribution! =)

jwittner
Josh Wittner (jwittner) requested changes Aug 10, 2018
View reviewed changes
Comment thread UnitySetup/UnitySetup.psm1
}
}

Start-BitsTransfer -Source $downloadSource -Destination $downloadDest

@jwittner Josh Wittner (jwittner) Aug 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is actually downloading multiple objects in one call as Start-BitsTransfer can take a list for the source/destination. I think you'll need to update the surrounding context a bit for this code to maybe kick all the downloads and then wait on them or iterate over them and sync download them one at a time.

@Ziugy Robert Onulak (Ziugy) Aug 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be good to see if we can determine if Bits is unable to run and have WebClient as the fallback too. Bits does run faster than WebClient, if available.

@jwittner Josh Wittner (jwittner) Aug 29, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you point to the data on the speed difference Robert Onulak (@Ziugy) ?

@Ziugy Robert Onulak (Ziugy) Aug 29, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only looked up a single data point before. Here's the first site I saw: https://blog.jourdant.me/post/3-ways-to-download-files-with-powershell

...though looking back at the numbers here 1 second seems insignificant.

Found another showing multiple other options with different results: https://powershellone.wordpress.com/2015/10/27/review-of-methods-to-download-files-using-powershell

@nickhorvath Nick Horvath (nickhorvath) Aug 31, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in the end you guys let me know - i agree the sequential nature of downloading would be slower, but i'm not sure how often multiple installers are pulled.. for us it'll always be 1.

@nickhorvath

Nick Horvath (nickhorvath) commented Aug 31, 2018

Copy link
Copy Markdown
Contributor Author

I've tested with the following scenarios:
single installer
$UnityVersion = "2018.2.2f1"
$InstallPath="c:\Program Files\Unity"
$unityInstall = Find-UnitySetupInstaller -Version $UnityVersion
$UnityInstallPath = -join($InstallPath, $UnityVersion)
$unityInstall | Install-UnitySetupInstance -Destination $UnityInstallPath

multiple installers
Install-UnitySetupInstance -Installers (Find-UnitySetupInstaller -Version '2017.3.0f3')

Ziugy
Robert Onulak (Ziugy) requested changes Aug 31, 2018
View reviewed changes
Comment thread UnitySetup/UnitySetup.psm1 Outdated
@nickhorvath Nick Horvath (nickhorvath) changed the title flip from start-bits to webclient. remove unused param flip from start-bits to webclient. remove unused param - resolves #119 Aug 31, 2018
jwittner
Josh Wittner (jwittner) requested changes Sep 4, 2018
View reviewed changes

@jwittner Josh Wittner (jwittner) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please bump the version to 5.0 as this changes the public API.

@jwittner
Josh Wittner (jwittner) merged commit 0704262 into microsoft:develop Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ziugy Robert Onulak (Ziugy) Ziugy approved these changes
@jwittner Josh Wittner (jwittner) jwittner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nickhorvath @jwittner @Ziugy