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

Reloading the plugin should also reload the underlying lib #33

Closed
gacarrillor opened this issue May 20, 2024 · 5 comments · Fixed by #34
Closed

Reloading the plugin should also reload the underlying lib #33

gacarrillor opened this issue May 20, 2024 · 5 comments · Fixed by #34

Comments

@gacarrillor
Copy link
Contributor

gacarrillor commented May 20, 2024

Otherwise, users would need to restart QGIS when ugrading the plugin to get the underlying lib up-to-date.

See #19 (comment)

@gacarrillor gacarrillor changed the title Reloading the pĺugin should also relload Reloading the pĺugin should also reload the underlying lib May 20, 2024
@gacarrillor gacarrillor changed the title Reloading the pĺugin should also reload the underlying lib Reloading the plugin should also reload the underlying lib May 20, 2024
@gacarrillor
Copy link
Contributor Author

gacarrillor commented May 27, 2024

Steps to reproduce the problem:

  1. Download https://github.com/opengisch/qgis-pg-service-parser-plugin/releases/download/0.1.7/pg_service_parser.0.1.7.zip (this version writes whitespaces between the = sign in the pg_service.conf file.)
  2. Install the ZIP from 1) and make sure it is activated (i.e., that you see the plugin icon in the plugins toolbar).
  3. Use the plugin to write on the pg_service.conf file.
  4. Verify the spaces between the = sign.
  5. Go to Plugin Manager -> All, search for pg service. Select the plugin from the list and click on Upgrade plugin.
  6. Repeat step 3).
  7. You'll still get spaces between the = sign, even if the version 0.1.9 (the one the plugin was upgraded to) already has a fix for that.

For reference, there are also steps to reproduce the solution to the problem.

@3nids
Copy link
Member

3nids commented May 27, 2024

for reference there was a discussion recently on the ML, with a potential solution: https://www.mail-archive.com/[email protected]/msg56034.html

he pointed out this upstream ticket: borysiasty/plugin_reloader#37
but as far as I understand, it's not a problem in plugin reloader but also on QGIS side

@3nids
Copy link
Member

3nids commented May 27, 2024

Hi Gerald,
you may use "from importlib import reload" to reload the packages that have been changed.
https://docs.python.org/3/library/importlib.html#importlib.reload
Greetings,
Benjamin

@gacarrillor
Copy link
Contributor Author

gacarrillor commented May 27, 2024

Yeah, I've been following the discussion and in fact contacted Gerald internally to test our solution on his issue. Up to now, I've been unable to replicate his problem, that's why I haven't posted to the ML.

but as far as I understand, it's not a problem in plugin reloader but also on QGIS side

In my opinion, QGIS already does a good job at it, and, with Gerald's issue, I'm attempting to see if Plugin Reloader also does its job in this regard. For me it might be more on plugin devs part to follow some guidelines to structure their plugins.

If the test I'm doing with Gerald goes well, I'd be willing to write something in this line in the QGIS docs.

Hi Gerald,
you may use "from importlib import reload" to reload the packages that have been changed.
https://docs.python.org/3/library/importlib.html#importlib.reload
Greetings,
Benjamin

Note that with PR #34, we don't need this. In fact, there are lots of issues along the road with these reload approaches.
For instance, see https://pyunit.sourceforge.net/notes/reloading.html

In summary, QGIS already handles well the reload, PR #34 helps to prove it.

@3nids 3nids closed this as completed in #34 May 27, 2024
@3nids
Copy link
Member

3nids commented May 27, 2024

Great, good work!

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 a pull request may close this issue.

2 participants