entrouvert
/
hobo
16
0
Fork 0
Code Issues Pull Requests 18 Releases Activity

add builtin cache to requests wrapper (#80246) #68

Open
pmarillonnet wants to merge 1 commits from wip/80246-requests-wrapper-cached-results into main
pull from: wip/80246-requests-wrapper-cached-results
merge into: entrouvert:main
Conversation 0 Commits 1 Files Changed 3 +99 -1
pmarillonnet commented 2023-08-21 10:25:55 +02:00
Owner
Copy Link
No description provided.
pmarillonnet added 1 commit 2023-08-21 10:25:56 +02:00
gitea/hobo/pipeline/head This commit looks good Details
4517823765 add builtin cache to requests wrapper (#80246)
pmarillonnet force-pushed wip/80246-requests-wrapper-cached-results from 4517823765 to 37b1aa9e41 2023-08-21 11:31:58 +02:00 Compare
pmarillonnet force-pushed wip/80246-requests-wrapper-cached-results from 37b1aa9e41 to 3232789116 2023-08-21 11:42:35 +02:00 Compare
pmarillonnet force-pushed wip/80246-requests-wrapper-cached-results from 3232789116 to 89006475e2 2023-08-21 11:57:55 +02:00 Compare
pmarillonnet changed title from WIP: add builtin cache to requests wrapper (#80246) to add builtin cache to requests wrapper (#80246) 2023-08-21 12:06:50 +02:00
nroche reviewed 2023-08-21 14:42:47 +02:00
hobo/requests_wrapper.py Outdated
@ -65,0 +100,4 @@
if method == 'GET' and cache_duration:
cache.set('%s_called' % cache_key, True, cache_duration)
response = super().request(method, url, **kwargs)
if method == 'GET' and cache_duration:
nroche commented 2023-08-21 13:00:54 +02:00
Owner
Copy Link

Je vois bien que c'est le même code dans combo, mais au risque de dire une bêtise, ce ne serait pas plutôt
invalidate_cache qu'il faudrait tester ligne 103 ?

Je vois bien que c'est le même code dans combo, mais au risque de dire une bêtise, ce ne serait pas plutôt invalidate_cache qu'il faudrait tester ligne 103 ?
pmarillonnet commented 2023-08-21 14:49:49 +02:00
Author
Owner
Copy Link

Non, le rôle du invalidate_cache est rempli plus haut, pour éviter de renvoyer le contenu du cache quand même bien celui-ci est connu d’hobo :

        if method == 'GET' and cache_duration:
            # handle cache
            cache_key = self.get_cache_key(url=url, params=kwargs.get('params', {}))
            cache_content = cache.get(cache_key)
            if cache_content and not invalidate_cache:  # <- ici
                response = Response()
                response.status_code = 200
                response.raw = BytesIO(smart_bytes(cache_content))
                return response
Non, le rôle du `invalidate_cache` est rempli plus haut, pour éviter de renvoyer le contenu du cache quand même bien celui-ci est connu d’hobo : ```python if method == 'GET' and cache_duration: # handle cache cache_key = self.get_cache_key(url=url, params=kwargs.get('params', {})) cache_content = cache.get(cache_key) if cache_content and not invalidate_cache: # <- ici response = Response() response.status_code = 200 response.raw = BytesIO(smart_bytes(cache_content)) return response ```
nroche commented 2023-08-21 15:15:24 +02:00
Owner
Copy Link

Merci pour l'explication.
En regardant mieux, toujours pour cette même ligne, sais-tu pourquoi il faut déposer un contenu factice dans le cache avant de faire la requête puis l'invalider ?
Côté combo c'est #79501 qui l'exploite, mais je pense qu'ici ça ajoute du code mort.

Merci pour l'explication. En regardant mieux, toujours pour cette même ligne, sais-tu pourquoi il faut déposer un contenu factice dans le cache avant de faire la requête puis l'invalider ? Côté combo c'est #79501 qui l'exploite, mais je pense qu'ici ça ajoute du code mort.
pmarillonnet commented 2023-08-21 15:24:30 +02:00
Author
Owner
Copy Link

Oui tu as raison, ce n’est sans doute pas nécessaire de maintenir cela dans hobo, on ne s’en servira pas. J’ai retiré cela.

Oui tu as raison, ce n’est sans doute pas nécessaire de maintenir cela dans hobo, on ne s’en servira pas. J’ai retiré cela.
nroche marked this conversation as resolved
pmarillonnet force-pushed wip/80246-requests-wrapper-cached-results from 89006475e2 to 37be5c3a74 2023-08-21 15:23:52 +02:00 Compare
nroche reviewed 2023-08-21 16:00:49 +02:00
hobo/requests_wrapper.py Outdated
@ -65,0 +90,4 @@
elif raise_if_not_cached:
raise NothingInCacheException()
if remote_service == 'auto':
nroche commented 2023-08-21 16:00:07 +02:00
Owner
Copy Link

ligne 93: Tu sais à quoi correspond ce 'auto' ?
ligne 96: Le code d'origine gérait déjà remote_service et kwargs['auth'].

Dans combo le 'auto' provient d'un paramètre passé à request quand on attend une signature.
Bref j'ai l'impression qu'il y a redondance à la ligne 94
et retrait de la signature lorsque l'on ne va pas vers un service (ligne 96), ce qui s'entend.
Peut-être ajouter un test là-dessus pour être plus explicite ?

ligne 93: Tu sais à quoi correspond ce 'auto' ? ligne 96: Le code d'origine gérait déjà remote_service et kwargs['auth']. Dans combo le 'auto' provient d'un paramètre passé à request quand on attend une signature. Bref j'ai l'impression qu'il y a redondance à la ligne 94 et retrait de la signature lorsque l'on ne va pas vers un service (ligne 96), ce qui s'entend. Peut-être ajouter un test là-dessus pour être plus explicite ?
pmarillonnet commented 2023-08-22 10:14:46 +02:00
Author
Owner
Copy Link

Oui tu as raison, j’ai viré tout cela, et ai laissé cette partie là du code dans sa version d’origine.

Oui tu as raison, j’ai viré tout cela, et ai laissé cette partie là du code dans sa version d’origine.
nroche marked this conversation as resolved
pmarillonnet force-pushed wip/80246-requests-wrapper-cached-results from 37be5c3a74 to b49e4a5d07 2023-08-22 10:14:27 +02:00 Compare
nroche approved these changes 2023-08-30 17:51:09 +02:00
All checks were successful
gitea/hobo/pipeline/head This commit looks good
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b wip/80246-requests-wrapper-cached-results main
git pull origin wip/80246-requests-wrapper-cached-results

Step 2:

Merge the changes and update on Gitea.
git checkout main
git merge --no-ff wip/80246-requests-wrapper-cached-results
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
nroche
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/hobo#68
No description provided.
Delete Branch "wip/80246-requests-wrapper-cached-results"

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?