entrouvert/wcs
entrouvert
/
wcs
14
1
Fork 0
Code Issues Pull Requests 34 Releases Activity

settings: add geocoding base URL to UI settings (#71997) #847

Merged
fpeters merged 1 commits from wip/71997-geocoding-settings into main 2023-11-26 12:47:51 +01:00
Conversation 2 Commits 1 Files Changed 3 +70 -4
fpeters commented 2023-11-19 15:58:33 +01:00
Owner
Copy Link
No description provided.
fpeters changed title from WIP: settings: add geocoding base URL to UI settings (#71997) to settings: add geocoding base URL to UI settings (#71997) 2023-11-19 16:35:40 +01:00
fpeters commented 2023-11-19 16:36:53 +01:00
Author
Owner
Copy Link

Le ticket parlait de geocoding_service_url, reverse_geocoding_service_url, nominatim_reverse_zoom_level et nominatim_key mais en pratique (d'un grep sur les tenants), c'est suffisant de juste faire nominatim_url.

Le ticket parlait de geocoding_service_url, reverse_geocoding_service_url, nominatim_reverse_zoom_level et nominatim_key mais en pratique (d'un grep sur les tenants), c'est suffisant de juste faire nominatim_url.
tnoel requested changes 2023-11-19 17:23:12 +01:00
wcs/admin/settings.py Outdated
@ -1156,0 +1161,4 @@
or get_publisher().get_site_option('nominatim_url'),
required=False,
hint=_('It will be suffixed by /search for geocoding and /reverse for reverse-geocoding.'),
)
tnoel commented 2023-11-19 17:21:41 +01:00
Owner
Copy Link

Mais il y a les possibles « self.get_site_option('geocoding_service_url') » et « self.get_site_option('reverse_geocoding_service_url') » qui peuvent exister et feront que cette option ne sera pas prise en charge.

Je me dis que si une des options geocoding_service_url / reverse_geocoding_service_url / nominatim_url existe, on devrait ne pas proposer de paramétrage, juste afficher les options et dire qu'elles sont configurées statiquement. Charge à celui qui voit ça de prévenir un tech qu'il faut retirer les affaires de site-options.cfg pour permettre une configuration correcte via l'UI.

Mais il y a les possibles « self.get_site_option('geocoding_service_url') » et « self.get_site_option('reverse_geocoding_service_url') » qui peuvent exister et feront que cette option ne sera pas prise en charge. Je me dis que si une des options geocoding_service_url / reverse_geocoding_service_url / nominatim_url existe, on devrait ne pas proposer de paramétrage, juste afficher les options et dire qu'elles sont configurées statiquement. Charge à celui qui voit ça de prévenir un tech qu'il faut retirer les affaires de site-options.cfg pour permettre une configuration correcte via l'UI.
fpeters commented 2023-11-19 17:34:03 +01:00
Author
Owner
Copy Link

Oui mais geocoding_service_url/reverse_geocoding_service_url ne sont jamais utilisées (d'un grep rapide), donc j'ai préféré faire sans. Oui je pourrais ajouter un message / désactiver le champ si elles sont définies (édit : fait), par contre (réponse au point suivant) je préfère garder la priorité à ce qui est défini dans l'UI, ça fluidifie la transition.

Oui mais geocoding_service_url/reverse_geocoding_service_url ne sont jamais utilisées (d'un grep rapide), donc j'ai préféré faire sans. Oui je pourrais ajouter un message / désactiver le champ si elles sont définies (édit : fait), par contre (réponse au point suivant) je préfère garder la priorité à ce qui est défini dans l'UI, ça fluidifie la transition.
fpeters commented 2023-11-20 09:31:30 +01:00
Author
Owner
Copy Link

priorité à ce qui est défini dans l'UI

Et pour un argument supplémentiare, c'est aussi ce qui est fait pour l'autre paramétrage de cette page, la position par défaut :

    def get_default_position(self):
        default_position = self.cfg.get('misc', {}).get('default-position', None)
        if not default_position:
            default_position = self.get_site_option('default_position')
        if not default_position:
            default_position = '50.84;4.36'
        return default_position
> priorité à ce qui est défini dans l'UI Et pour un argument supplémentiare, c'est aussi ce qui est fait pour l'autre paramétrage de cette page, la position par défaut : ``` def get_default_position(self): default_position = self.cfg.get('misc', {}).get('default-position', None) if not default_position: default_position = self.get_site_option('default_position') if not default_position: default_position = '50.84;4.36' return default_position ```
tnoel marked this conversation as resolved
wcs/qommon/publisher.py Outdated
@ -838,1 +838,3 @@
url = self.get_site_option('nominatim_url') or 'https://nominatim.entrouvert.org'
url = (
get_cfg('misc', {}).get('geocoding-services-base-url')
or self.get_site_option('nominatim_url')
tnoel commented 2023-11-19 17:23:03 +01:00
Owner
Copy Link

Par rapport à ce que je dis plus haut, on mettrait plutôt le get_site_option en premier ; s'il existe il ne peut pas y avoir de surcharge par l'interface des settings.

Par rapport à ce que je dis plus haut, on mettrait plutôt le get_site_option en premier ; s'il existe il ne peut pas y avoir de surcharge par l'interface des settings.
tnoel marked this conversation as resolved
wcs/qommon/publisher.py Outdated
@ -851,1 +855,3 @@
url = self.get_site_option('nominatim_url') or 'https://nominatim.entrouvert.org'
url = (
get_cfg('misc', {}).get('geocoding-services-base-url')
or self.get_site_option('nominatim_url')
tnoel commented 2023-11-19 17:23:09 +01:00
Owner
Copy Link

Idem.

Idem.
tnoel marked this conversation as resolved
fpeters force-pushed wip/71997-geocoding-settings from 5d7b78b858 to d3c751093f 2023-11-19 17:42:57 +01:00 Compare
fpeters commented 2023-11-19 17:44:26 +01:00
Author
Owner
Copy Link

Avec un gros avertissment si jamais il y a geocoding_service_url ou reverse_geocoding_service_url dans le site-options.cfg, ça donne :

image

Avec un gros avertissment si jamais il y a geocoding_service_url ou reverse_geocoding_service_url dans le site-options.cfg, ça donne : ![image](/attachments/0f85376d-6971-4a0d-bc97-b7d2923aa369)
image.png
329 KiB
fpeters requested review from tnoel 2023-11-23 09:57:24 +01:00
tnoel approved these changes 2023-11-24 17:20:51 +01:00
tnoel left a comment
Owner
Copy Link

Nickel.

Nickel.
fpeters merged commit 727e9bd5aa into main 2023-11-26 12:47:51 +01:00
fpeters deleted branch wip/71997-geocoding-settings 2023-11-26 12:47:51 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
tnoel
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/wcs#847
No description provided.
Delete Branch "wip/71997-geocoding-settings"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?