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

APIs required by Tensorboard ext (as well as Jupyter ext) #22174

Open
DonJayamanne opened this issue Oct 9, 2023 · 8 comments
Open

APIs required by Tensorboard ext (as well as Jupyter ext) #22174

DonJayamanne opened this issue Oct 9, 2023 · 8 comments
Labels
feature-request Request for new features or functionality needs proposal Need to make some design decisions partner ask

Comments

@DonJayamanne
Copy link

DonJayamanne commented Oct 9, 2023

There are a few requirements in Tensorboard (& Jupyter ext), and I will not create separate issues for each of the APIs as the solution/API exposed could end up being different.

Requirements

  • Ability to install packages
  • Ability to detect whether packages are installed
  • Ability to get version of packages
  • Ability to run Python code (spawning kernels, etc)

Proposed solutions

  • Provide an API to manage packages
  • Provide an API to return
    • Activated env vars of Python env
    • Paths to the tools such as conda, poetry, etc
  • Short term
    • Leave existing tensorboard package installation code in Python extension
    • Expose some command or the like that can be used by Tensorboard (to detect/install tensorboard package)
    • This temporary solution can be in place until we get an API sorted out,

Note:

  • Detecting whether a package is installed and getting their versions can be done with just a single api getActivatedEnvVars
  • Installation of packages is what is complex

Related issues

@karrtikr @karthiknadig /cc

@DonJayamanne DonJayamanne added the feature-request Request for new features or functionality label Oct 9, 2023
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Oct 9, 2023
@DonJayamanne DonJayamanne changed the title APIs required by Jupyter and Tensorboard ext APIs required by Tensorboard ext (as well as Jupyter ext) Oct 9, 2023
@karthiknadig karthiknadig self-assigned this Oct 10, 2023
@karthiknadig
Copy link
Member

The sort term solution looks fine.

Activated env vars of Python env

Is the current getEnvironmentVariables public API not enough here?

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 11, 2023
@DonJayamanne
Copy link
Author

Is the current getEnvironmentVariables public API not enough here?

Nope, that only returns env varialbes from the .env file, not from the active Python environment.
I need one that returns variables from an activated python environment.

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Oct 11, 2023
@karthiknadig
Copy link
Member

@karrtikr I thought you updated this to include the .venv env variables also?

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 11, 2023
@karrtikr
Copy link

Nope, we do not know for sure how activation will look until experiment is finalized, hence we haven't exposed the APIs yet.

@DonJayamanne
Copy link
Author

I too thought it was done, and I feel the method getEnvironmentVariables doesn't belong under environments or should be renamed (too late for either one).
I think @karthiknadig too made the same mistake

Unless of course one reads the comments

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Oct 12, 2023
@DonJayamanne
Copy link
Author

ok until experiment is finalized, hence we haven't exposed the APIs yet.

Please can you point me to that code,
Interested in that, thanks

@karrtikr
Copy link

I think I mentioned before in slack that exposing such an API is blocked on #20950 where we're actively working with VS Code to finalize how activation will look like, which determines the shape of the API.

If we find out using activated environment variables is sufficient, we can expose getActivatedEnvironmentVariables API, which will be asynchronous as opposed to getEnvironmentVariables API.

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 12, 2023
@karrtikr karrtikr removed the info-needed Issue requires more information from poster label Oct 12, 2023
@karthiknadig
Copy link
Member

I think having two APIs for getting environment variables can get confusing. We can discuss this offline.

@karthiknadig karthiknadig added needs proposal Need to make some design decisions partner ask and removed triage-needed Needs assignment to the proper sub-team labels Jan 18, 2024
@karthiknadig karthiknadig removed their assignment Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality needs proposal Need to make some design decisions partner ask
Projects
None yet
Development

No branches or pull requests

3 participants