Azure-Samples / ms-identity-python-webapp

A Python web application calling Microsoft graph that is secured using the Microsoft identity platform
MIT License
284 stars 135 forks source link

Update instructions for running configure.ps1 script #43

Closed idg-sam closed 3 years ago

idg-sam commented 3 years ago

configure.ps1 script causes an error as per https://github.com/Azure-Samples/ms-identity-python-webapp/issues/24 when run from project root directory.

rayluo commented 3 years ago

Thanks! @abhidnya13 is more familiar with this script than me. I'll defer this to @abhidnya13 to verify it.

abhidnya13 commented 3 years ago

Hey @idg-sam this looks like a quick fix to temporarily solve the issue. However, I think we should investigate on why configure.ps1 does not have the right value ? This configure.ps1 was generated using @jmprieur App Creation scripts generator and it uses the value in sample.json. Can we investigate this a little more to see if these scripts need to be changed instead or are updates to sample.json needed ?

idg-sam commented 3 years ago

Hi @abhidnya13 , as the script generator currently stands, whatever we put in the sample.json, the generated ps1 script will have to be opinionated about which $pwd.Path (directory) the user initiates the script from.

We ask the user to run the following in our other samples in order to generate apps and to modify config files in the parent directory relative to the $pwd.Path:

cd .\AppCreationScripts\
.\Configure.ps1

Shall we go ahead and change the README.md for now and then we can look into making a pull request to the script generator?

I'll bring up an improvement request in our meeting today, asking that we modify the script generator to create scripts that use $PSScriptRoot + "\\......" (location of ps1 script being run) rather than $pwd.Path+ "\\......."(directory of current terminal window) as the base path. Thus the script won't have to be opinionated about which directory it is run from.

kalyankrishna1 commented 3 years ago

Hi, you need to be in the /Appcreationscript folder to run the script. That's why we advise the same in readme and elsewhere. We had a similar problem with .NET too and there was a minor debate about whether the scripts should be run from a directory above or within it. After a few scenarios like creation of an html file and some scripts actually need the developer to modify first (not very strong arguments, but arguments still), we decided to "standardize" on scripts being run from that folder itself..

idg-sam commented 3 years ago

We should ideally do this changes in the other MSAL Python samples as well if they dont already do this. Can you verify this @idg-sam ?

Ok, I'm on it this week.