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

update url for fetching plugin listing from napari hub #313

Merged
merged 6 commits into from
Sep 25, 2023

Conversation

nclack
Copy link
Collaborator

@nclack nclack commented Sep 15, 2023

This PR updates for a couple api changes in dependencies that are blocking #311 and #312.

For the napar-hub api, the /plugins endpoint was removed (see here). This impacts the get_hub_plugins() function, and a test test_get_hub_plugins which fails without this change. It doesn't appear that anything actually uses these, so this PR removes them.

Also build updated to 1.0. IsolatedEnvBuilder was renamed to DefaultIsolatedEnv.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #313 (84de569) into main (db7d7c5) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #313   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2818      2814    -4     
=========================================
- Hits          2818      2814    -4     
Files Coverage Δ
src/npe2/_inspection/_fetch.py 100.00% <ø> (ø)
src/npe2/_inspection/_full_install.py 100.00% <100.00%> (ø)

@nclack nclack marked this pull request as ready for review September 15, 2023 23:08
@Czaki
Copy link
Collaborator

Czaki commented Sep 16, 2023

Is it deprecated or removed?

@nclack
Copy link
Collaborator Author

nclack commented Sep 16, 2023

Is it deprecated or removed?

removed. I'll update the description.

@nclack nclack requested a review from Czaki September 16, 2023 16:41
@Czaki
Copy link
Collaborator

Czaki commented Sep 16, 2023

Is it impossible to restore it on few months? To have migration time?

@nclack
Copy link
Collaborator Author

nclack commented Sep 16, 2023

Is it impossible to restore it on few months? To have migration time?

I created chanzuckerberg/napari-hub#1263 to request this.

As far as I can tell [1], no one uses the get_hub_plugins() function and it's behind a private interface. So I'm not too worried about backward compatibility here. But there are lots of good reasons to use a deprecation window just in case.

Since it looks like nothing uses get_hub_plugins() an alternative to this pr is to remove get_hub_plugins() and the corresponding test.

[1]: I did a code search through npe2, napari and npe2api.

@Czaki
Copy link
Collaborator

Czaki commented Sep 17, 2023

If it is not used, then it should be removed. The napari-plugin-manager should also be checked.

@nclack
Copy link
Collaborator Author

nclack commented Sep 18, 2023

@Czaki I didn't see any use in napari-plugin-manager either. I removed the call and test like you suggested. Also updated the PR description. Let me know if you think of anything else. Getting this in will help #311 and #312 to move forward.

@nclack
Copy link
Collaborator Author

nclack commented Sep 18, 2023

@aganders3 can you double-check that we're not using the get_hub_plugins() function anywhere?

@aganders3
Copy link
Contributor

I also don't see this used anywhere in napari or the plugin manager.

There was a function in the plugin manager using this hub endpoint, but it was not using the npe2 function to reach it anyway. That whole file was removed in napari 0.4.18 and prior to the plugin manager being extracted to its own repo.

@nclack nclack requested review from Czaki and DragaDoncila and removed request for Czaki and DragaDoncila September 25, 2023 18:02
@nclack nclack merged commit bd0d46c into napari:main Sep 25, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants