Code Monkey home page Code Monkey logo

Comments (16)

chrmarti avatar chrmarti commented on May 18, 2024 1

Continued in #53. Thanks @jdneo.

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Hi @chrmarti , I sent out a PR about this issue: #48. 😄

from vscode-azure-account.

chrmarti avatar chrmarti commented on May 18, 2024

I have so far taken the view that when a user wants to do something (e.g., Open Cloud Shell) that requires the user to sign in first, it makes sense to let the user repeat the initial action (Open Cloud Shell) because there is such a long time between taking that initial action and completing the sign in process. The user might simply have forgotten what s/he was up to by the time the sign in completes.

Could you describe the scenario you are looking at?

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Yep, the scenario is just like you said.

  1. User clicks "Open Cloud Shell".
  2. User is told that he needs to sign in first.
  3. User signs in.
  4. Cloud Shell is provisioned.
  5. Cloud Shell is shown to the user.

I agree that this is not a problem if users just forget their intention after signing in. But what if the user still remember? He might keep waiting but nothing happens.

I think continue the execution might improve the experience for the latter users, as long as this change won't introduce regression. 😄

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Em, actually this feedback is from our testers. After they click open cloud shell, nothing happens after signing in, which makes them confused.

BTW, an idea just came up in my mind.

Maybe I can check the login status through the API, and if the user is not signed in, tell him that he needs to sign in first, and then trigger the login command from the account extension.

After login, next time user trigger cloud shell related command, he can pass the login status check and do whatever he wants to do.

But I still much prefer the one click experience, since less action is needed.

What do you think?

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

@chrmarti I saw your code change and tested on my side. I like the experience that telling user sign in needed in the terminal panel. Thanks!

But when user hasn't signed in at begining, I got Entry not found in cache. error when calling acquireToken(). Is that becaus I missed something?

from vscode-azure-account.

chrmarti avatar chrmarti commented on May 18, 2024

I cannot reproduce the "Entry not found in cache" error. Do you see it every time you start without being signed in?

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Yes it is. The problem only occurs if I am not signed in.

My code looks like this:

const accountAPI: AzureAccount = vscode.extensions
    .getExtension<AzureAccount>("ms-vscode.azure-account")!.exports;
this.cloudShell =  accountAPI.createCloudShell("Linux");
this.terminal = await this.cloudShell.terminal;
this.terminal.show();

const session: AzureSession = await cloudShell.session;
const token: IToken = await acquireToken(session);

The method acquireToken() is just copied from the code in azure account extension

from vscode-azure-account.

chrmarti avatar chrmarti commented on May 18, 2024

I do just that before resolving the cloudShell.session promise and there it seems to work: https://github.com/Microsoft/vscode-azure-account/blob/f0846fc601554ddb3f945e34e6a85551b705fca7/src/cloudConsole.ts#L310

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Do you mean I need to pass the Promise to the method? But the acquireToken() is declared to accept AzureSession type.

from vscode-azure-account.

chrmarti avatar chrmarti commented on May 18, 2024

The way you have it should work. I'm surprised it does not work because my code is doing the same shortly before your code does it. Maybe put breakpoints in your code and here to step through: https://github.com/Microsoft/vscode-azure-account/blob/f0846fc601554ddb3f945e34e6a85551b705fca7/src/cloudConsole.ts#L310

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Hi @chrmarti , I did some tests and here is my observation:

  1. Open cloud shell command in the Azure Account extension works fine no matter I'm signed in or not.
  2. If I have signed in, open cloud shell using my above code snippet works fine.
  3. If I haven't signed in at beginning, I will get "Entry not found in cache" error.

I find there is a difference is in the session.credentials.context.cache.

When there is no error, there are two entries in the cache, one for the common tenant, the other for the Microsoft AD
When there occurs error, there are three entries in the cache, one for the common tenant, the other two are the same, both of them are the Microsoft AD. See my screenshot below.

screen shot 2018-04-05 at 8 49 26 pm

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Hi @chrmarti, I have a question for the method createCloudConsole(): https://github.com/Microsoft/vscode-azure-account/blob/master/src/cloudConsole.ts#L156

Can we resolve the Promise for AzureSession before we calling the acquireToken()? for example:

	...
	deferredSession!.resolve(pick.session);	
	token = await acquireToken(pick.session);
} else {
	deferredSession!.resolve(api.sessions[0]);
	token = await acquireToken(api.sessions[0]);
}

I tested locally and seems this can resolve the error for Entry not found in cache

from vscode-azure-account.

chrmarti avatar chrmarti commented on May 18, 2024

I'm not opposed to shuffle things around, but I'd rather first understand why it sometimes works and sometimes doesn't. It looks suspiciously like a timing issue at the moment and I'm not sure the reshuffling will fix it or just make it less likely to happen.

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

It looks like not a timing issue.

api.sessions[0] is different from result.token.session from my observation.

from vscode-azure-account.

jdneo avatar jdneo commented on May 18, 2024

Hi @chrmarti I found out a way to reproduce Entry not found in cache error. Just trigger the Azure: Open bash in clould shell twice.

More details can be found in the gif below:

report

from vscode-azure-account.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.