microsoft / oxa-tools

Open edX on Azure Tools
MIT License
36 stars 103 forks source link

Apply Cli version configuration override during storage container creation #358

Closed eltoncarr closed 6 years ago

eltoncarr commented 6 years ago

What does this PR do? Please provide some context

Apply configuration override for Azure Cli version for storage account creation

Where should the reviewer start?

The changes.

How can this be manually tested? (brief repro steps and corpnet-URL with change)

Run powershell -f Create-StorageContainer.ps1 (with the appropriate parameters)

Without the changes, the setup fails with Login Failed | const { required, nullable } = mapper; With the changes, the error goes away.

Additional context: It seems there was a recent update to the ms-rest npm package. The update was done 6/21.

Here’s the error: /usr/local/lib/node_modules/azure-cli/node_modules/ms-rest/lib/serialization.js:78 const { required, nullable } = mapper; ^ SyntaxError: Unexpected token { at exports.runInThisContext (vm.js:53:16) at Module._compile (module.js:374:25) at Module.pModule._compile (/usr/local/lib/node_modules/azure-cli/node_modules/streamline/lib/compiler/register.js:72:15) at Object.Module._extensions..js (module.js:417:10) at Module.load (module.js:344:32) at Function.Module._load (module.js:301:12) at Module.require (module.js:354:17) at require (internal/module.js:12:17) at Object.<anonymous> (/usr/local/lib/node_modules/azure-cli/node_modules/ms-rest/lib/webResource.js:11:20) at Module._compile (module.js:410:26)

So the change is opportunistic. We are simply moving to Cli2 instead since it is newer and preferred.

What are the relevant TFS items? (list id numbers)

N/A

Definition of done:

Reminders DURING merge

  1. If you're merging from a short-term (feature) branch into a long-term branch (like dev, release, or master) then "Squash and merge" to keep our history clean.
  2. If merging from two longterm branches (like cherry picks from upstream, dev to release, etc) then "Create merge commit" to preserve individual commits.
manikarthikk commented 6 years ago

@eltoncarr , Change Looks good. As part of our branching strategy, we should create the PR against oxa/dev.fic and send it to release and then to ms.fic.

kevsers commented 6 years ago

please cherry-pick these into oxa/dev.fic as well.

larryuribe commented 6 years ago

So, do you are resolving the issue?, and wil be merge with new build of oxa-tools. And later we need just git clone again and redeploy???

When this issue will be updated? I'm now with Latam Lex team compromised for release a new site ASAP!...

:(

eltoncarr commented 6 years ago

@larryuribe, I have updated the issue