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.

Sign up for GitHub

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

Jump to bottom

Add support for starting TensorBoard session with a remote log directory via URL #16477

Merged
merged 3 commits into from
Jul 13, 2021
Merged

Add support for starting TensorBoard session with a remote log directory via URL #16477

merged 3 commits into from
Jul 13, 2021

Conversation

joyceerhl
Copy link

For #16461

The PyTorch profiler plugin now accepts a remote log directory for profiler logs. This allows our users to leverage that support when creating TensorBoard sessions.

image

image

@joyceerhl joyceerhl marked this pull request as ready for review June 14, 2021 20:18
rchiodo
rchiodo reviewed Jun 14, 2021
View reviewed changes
src/client/tensorBoard/tensorBoardSession.ts
@@ -331,6 +341,10 @@ export class TensorBoardSession {
case selectAFolder:
case selectAnotherFolder:
return this.showFilePicker();
case enterRemoteUrl:
return this.applicationShell.showInputBox({
prompt: TensorBoard.enterRemoteUrlDetail(),
Copy link

Choose a reason for hiding this comment

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

Should there be any validation on the url? I believe we do that for jupyter URIs. Not sure if there's a standard form for tensorboard remote URIs

Copy link
Author

Choose a reason for hiding this comment

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

I was debating this. This can be any web URL AFAIK. I didn't really want to roll my own validation here. One thought I had was since VS Code has a URI implementation that can parse URLs, we could either hand off the URL to their Uri.parse function and check if the key properties are empty string, or duplicate the regex and logic they use for validation. But it would probably also work to just reuse our existing validation logic.

IanMatthewHuff
IanMatthewHuff reviewed Jun 14, 2021
View reviewed changes
package.nls.json Outdated Show resolved Hide resolved
@brettcannon
Copy link
Member

@joyceerhl can you either merge or close this PR?

@joyceerhl joyceerhl merged commit 1d932b9 into microsoft:main Jul 13, 2021
karthiknadig pushed a commit to karthiknadig/vscode-python that referenced this pull request Jul 14, 2021
@joyceerhl @karthiknadig
Add support for starting TensorBoard session with a remote log direct…
711a9a7 
…ory via URL (microsoft#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo
karthiknadig added a commit that referenced this pull request Jul 16, 2021
@karthiknadig @dependabot
@joyceerhl @kimadeline
Cherry pick fixes into release (#16686)
03df7c2 
* Bump actions/setup-node from 2.1.5 to 2.2.0 (#16592)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.1.5 to 2.2.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.5...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for starting TensorBoard session with a remote log directory via URL (#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo

* Bump isort from 5.8.0 to 5.9.2 (#16636)

Bumps [isort](https://github.com/pycqa/isort) from 5.8.0 to 5.9.2.
- [Release notes](https://github.com/pycqa/isort/releases)
- [Changelog](https://github.com/PyCQA/isort/blob/main/CHANGELOG.md)
- [Commits](PyCQA/isort@5.8.0...5.9.2)

---
updated-dependencies:
- dependency-name: isort
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make getInterpreters() API faster for subsequent calls (#16674)

* Change the way auto-selection works (#16644)

* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add new auto-selection logic

* Add tests for getEnvTypeHeuristic

* Move persistent store initialization back out

* Update tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
karthiknadig added a commit that referenced this pull request Jul 22, 2021
@karthiknadig @dependabot
@joyceerhl @kimadeline
Merge release back into main (#16763)
6314e70 
* Fix 'Cannot read property 'resolveEnv' of undefined' error (#16677)

* Cherry pick fixes into release (#16686)

* Bump actions/setup-node from 2.1.5 to 2.2.0 (#16592)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.1.5 to 2.2.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.5...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for starting TensorBoard session with a remote log directory via URL (#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo

* Bump isort from 5.8.0 to 5.9.2 (#16636)

Bumps [isort](https://github.com/pycqa/isort) from 5.8.0 to 5.9.2.
- [Release notes](https://github.com/pycqa/isort/releases)
- [Changelog](https://github.com/PyCQA/isort/blob/main/CHANGELOG.md)
- [Commits](PyCQA/isort@5.8.0...5.9.2)

---
updated-dependencies:
- dependency-name: isort
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make getInterpreters() API faster for subsequent calls (#16674)

* Change the way auto-selection works (#16644)

* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add new auto-selection logic

* Add tests for getEnvTypeHeuristic

* Move persistent store initialization back out

* Update tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change version for release (#16722)

* Ensure we block on autoselection when no interpreter is explictly set by user (#16723)

* Ensure we block on autoselection when no interpreter is explictly set by user

* Added tests

* News entry

* Update change log for release. (#16731)

* Fix autoselection when opening a python file directly (#16733)

* Fix autoselection when opening a python file directly

* Update changelog

* Add tests

* Ignore cache when querying for interpreters during auto-selection (#16734)

* Ignore cache when getting envs for autoselection

* Don't call autoSelectInterpreter twice

* Update debugger via point release (#16746)

* Update version

* Update change log

* Update wheels to 3.9 (#16745)

* Clean up

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
karthiknadig added a commit that referenced this pull request Jul 28, 2021
@karthiknadig @dependabot
@joyceerhl @kimadeline
Mergeback changes from point release to main (#16809)
eae6b48 
* Fix 'Cannot read property 'resolveEnv' of undefined' error (#16677)

* Cherry pick fixes into release (#16686)

* Bump actions/setup-node from 2.1.5 to 2.2.0 (#16592)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.1.5 to 2.2.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.1.5...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for starting TensorBoard session with a remote log directory via URL (#16477)

* Add support for remote logdirs

* Add missing keys

* Fix typo

* Bump isort from 5.8.0 to 5.9.2 (#16636)

Bumps [isort](https://github.com/pycqa/isort) from 5.8.0 to 5.9.2.
- [Release notes](https://github.com/pycqa/isort/releases)
- [Changelog](https://github.com/PyCQA/isort/blob/main/CHANGELOG.md)
- [Commits](PyCQA/isort@5.8.0...5.9.2)

---
updated-dependencies:
- dependency-name: isort
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make getInterpreters() API faster for subsequent calls (#16674)

* Change the way auto-selection works (#16644)

* New comparison logic

* Add experiment group

* Register and call it

* Add service registry tests

* Add interpreter selector unit tests

* Add comparison unit tests

* Add intepreter selector test

* News file

* Adjust comments

* Reuse getSortName

* Add new auto-selection logic

* Add tests for getEnvTypeHeuristic

* Move persistent store initialization back out

* Update tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change version for release (#16722)

* Ensure we block on autoselection when no interpreter is explictly set by user (#16723)

* Ensure we block on autoselection when no interpreter is explictly set by user

* Added tests

* News entry

* Update change log for release. (#16731)

* Fix autoselection when opening a python file directly (#16733)

* Fix autoselection when opening a python file directly

* Update changelog

* Add tests

* Ignore cache when querying for interpreters during auto-selection (#16734)

* Ignore cache when getting envs for autoselection

* Don't call autoSelectInterpreter twice

* Update debugger via point release (#16746)

* Update version

* Update change log

* Update wheels to 3.9 (#16745)

* Point release with debugpy fix (#16776)

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IanMatthewHuff IanMatthewHuff IanMatthewHuff approved these changes

@paulacamargo25 paulacamargo25 paulacamargo25 approved these changes

@rchiodo rchiodo rchiodo approved these changes

@kimadeline kimadeline kimadeline approved these changes

@DavidKutu DavidKutu Awaiting requested review from DavidKutu

@DonJayamanne DonJayamanne Awaiting requested review from DonJayamanne

Assignees

@kimadeline kimadeline

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@joyceerhl @brettcannon @IanMatthewHuff @paulacamargo25 @rchiodo @kimadeline