-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Fixes #12828] New remote datasets are not registered inside proxy al… #12829
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12829 +/- ##
===========================================
+ Coverage 55.87% 67.97% +12.10%
===========================================
Files 977 977
Lines 59035 59046 +11
Branches 6897 6899 +2
===========================================
+ Hits 32984 40138 +7154
+ Misses 24669 17264 -7405
- Partials 1382 1644 +262 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid using globals()
like this.
And I don't understand why changes do not modify the proxy_urls_registry
content, since it's a singleton instance at the module level.
Let's investigate this better @mattiagiupponi
f59360d
to
7e7234f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattiagiupponi we cannot rely on reading from memached (or, worst, the filesystem) on each request to the proxy. In the case of 3dtiles, for example, it would be a bottleneck that worsen the proxy performances even more.
To avoid implementing a signaling system between celery and django, we can take an optimistic approach: we add the URL to the proxy before the celery task is created by the importer, when we're still inside the django process.
In case the remote layer creation fails, for whatever reason, we will have a registered host that doesn't map to a layer, but I don't think it's a big problem.
We can have a mechanism that reloads the registry from the DB once in a while, to realign the registry to the DB.
For example, the time of the last registry initialization can be stored in the registry. On each request we can check the time and if the registry is stale (1 day?) we reinitialize the registry.
…lowed hosts when GeoNode runs asynchoronously
298cff0
to
4190e59
Compare
…lowed hosts when GeoNode runs asynchoronously
…lowed hosts when GeoNode runs asynchoronously
…lowed hosts when GeoNode runs asynchoronously
backport label removed, in case is required the backport must be done manually |
…lowed hosts when GeoNode runs asynchoronously
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.