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

Fix for localization errors and library versioning #6936

Merged
merged 5 commits into from
Apr 12, 2018

Conversation

keljos
Copy link
Member

@keljos keljos commented Apr 10, 2018

  • Updating the vsts-task-tool-lib and vsts-task-lib library versions to pull down a fix for a localization bug.

  • Updating the javaToolInstaller's usage of the azure blob storage downloadArtifacts method due to changes in the methods functionality.

  • Fixing an error in the datasource bindings that was causing the pick lists to not be properly filled in.

@keljos keljos requested review from jpricket and lkillgore April 10, 2018 19:19
@@ -43,12 +43,12 @@ async function getJava(versionSpec: string) {
if (version) { //This version of Java JDK is already in the cache. Use it instead of downloading again.
console.log(taskLib.loc('Info_ResolvedToolFromCache', version));
} else if (fromAzure) { //Download JDK from an Azure blob storage location and extract.
console.log(taskLib.loc('RetrievingJdkFromAzure', version));
Copy link
Member Author

Choose a reason for hiding this comment

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

This was an unused variable being passed into the localization message so I removed it.

compressedFileExtension = getFileEnding(taskLib.getInput('azureCommonVirtualFile', true));

const azureDownloader = new AzureStorageArtifactDownloader(taskLib.getInput('azureResourceManagerEndpoint', true),
taskLib.getInput('azureStorageAccountName', true), taskLib.getInput('azureContainerName', true), taskLib.getInput('azureCommonVirtualFile', false));
await azureDownloader.downloadArtifacts(extractLocation, '*' + compressedFileExtension);
taskLib.getInput('azureStorageAccountName', true), taskLib.getInput('azureContainerName', true), "");
Copy link
Member Author

Choose a reason for hiding this comment

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

The download artifacts method now treats the path as being a folder and adds a '/' on the end. The work around is to pass the filename in as the itemPattern.

@@ -24,7 +24,7 @@
"dependencies": {
"azure-arm-rest": "file:../../_build/Tasks/Common/azure-arm-rest-1.0.2.tgz",
"azure-blobstorage-artifactProvider": "file:../../_build/Tasks/Common/azure-blobstorage-artifactProvider-1.0.0.tgz",
"vsts-task-lib": "2.1.0",
"vsts-task-tool-lib": "^0.4.1"
"vsts-task-lib": "2.4.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Updating these libraries to include the fixes for localization bugs.

@@ -43,12 +43,12 @@ async function getJava(versionSpec: string) {
if (version) { //This version of Java JDK is already in the cache. Use it instead of downloading again.
console.log(taskLib.loc('Info_ResolvedToolFromCache', version));
} else if (fromAzure) { //Download JDK from an Azure blob storage location and extract.
console.log(taskLib.loc('RetrievingJdkFromAzure', version));
console.log(taskLib.loc('RetrievingJdkFromAzure'));
compressedFileExtension = getFileEnding(taskLib.getInput('azureCommonVirtualFile', true));
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a good idea to get this input taskLib.getInput('azureCommonVirtualFile', true) only once, save it in a variable, and use it again.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I missed that I was using it twice when I rearranged the code. I'll fix this.

taskLib.getInput('azureStorageAccountName', true), taskLib.getInput('azureContainerName', true), taskLib.getInput('azureCommonVirtualFile', false));
await azureDownloader.downloadArtifacts(extractLocation, '*' + compressedFileExtension);
taskLib.getInput('azureStorageAccountName', true), taskLib.getInput('azureContainerName', true), "");
await azureDownloader.downloadArtifacts(extractLocation, '*' + taskLib.getInput('azureCommonVirtualFile', false));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the '*' + needed? ('*.zip' makes sense to me; '*javatool.zip' seems a bit odd.)

Copy link
Member Author

Choose a reason for hiding this comment

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

The Azure library team who owns the downloadArtifacts functionality had suggested using the * in case the entire path was not entered. I can remove it if there are strong feelings against it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Either way is fine by me. It looks odd to me, but it doesn't look wrong.

@keljos keljos merged commit 961fb86 into master Apr 12, 2018
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.

3 participants