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

Bugfix gh38755 expose project setinstance #38758

Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Sep 15, 2020

Fixes #38755

Needs forward porting to all active branches

Fixes qgis#38755

Needs forward porting to all active branches
@elpaso elpaso added API API improvement only, no visible user interface changes Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Sep 15, 2020
@github-actions github-actions bot added this to the 3.11.0 milestone Sep 15, 2020
@nyalldawson
Copy link
Collaborator

I realise this is a regression fix, but it also makes me nervous in that a plugin calling this (outside of server) will cause major breakage to qgis app. Is there any alternative approach?

@elpaso
Copy link
Contributor Author

elpaso commented Sep 15, 2020

I expected this objection but I don't really see a big issue here, the advantage is that it adds some more flexibility to the singleton management, that IMO should eventually disappear completely (QGIS 4 maybe?).

I though about exposing it through a server specific API (QgsServer::setProjectInstance()?) but it doesn't really belong there: it is for server consumption but who knows: there might be other standalone scripts not related to serve that could benefit of that API.

Another alternative is expose it through the config cache with an other optional argument to QgsConfigCache const QgsProject *project( const QString &path, QgsServerSettings *settings = nullptr ) or with a new method.

@dmarteau
Copy link
Contributor

dmarteau commented Sep 15, 2020

there might be other standalone scripts not related to serve that could benefit of that API.

It might be the best alternative, since restoring the setInstance call in the QgsConfigCache only does not solve the issue: preventing to set the singleton breaks other behaviors (like expression/function evaluation) and has an impact on every python scripts that might use it.

that IMO should eventually disappear completely (QGIS 4 maybe?).

+1

@nyalldawson
Copy link
Collaborator

nyalldawson commented Sep 15, 2020

If you're going to expose it, please add a:

\warning calling this method can have serious, unintended consequences, including instability, data loss and undefined behaviour. Use with EXTREME caution!

@nyalldawson
Copy link
Collaborator

Fixes regression introduced in LTR => immediate merge

@nyalldawson nyalldawson merged commit 711faac into qgis:release-3_10 Sep 16, 2020
@elpaso elpaso deleted the bugfix-gh38755-expose-project-setinstance branch September 17, 2020 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API improvement only, no visible user interface changes Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants