Add redis stream #16

zteeed · 2019-08-11T02:15:27Z

No description provided.

bonnetn

la v2 se rapproche :D

worker/main.py

worker/requirements.in

worker/worker/__init__.py

worker/requirements.in

worker/main.py

api/api/constants.py

api/api/handlers.py

bonnetn · 2019-09-16T16:08:24Z

worker/main.py

+        if stream_name == REDIS_STREAM_CHALLENGES:
+            await set_all_challenges()
+
+        await app.redis.xack(stream_name, CG_NAME, message_id)


ya pas un mode autoack sur aioredis?

euh aucune idée jvais regarder

worker/worker/__init__.py

bonnetn · 2019-09-16T16:10:25Z

worker/worker/constants.py

-GITHUB_ACCOUNTS = ["https://github.com/zteeed", "https://github.com/bonnetn"]
+REDIS_STREAM_USERS = 'update_users'
+REDIS_STREAM_CHALLENGES = 'update_challenges'
+CG_NAME = 'rootme'


Tu peux pas merge api/api/constants.py et celui là? si tu mets les mêmes valeurs..

bah ca veut dire que les subprojects worker et api sont dépendants, a terme peut etre gérer ça avec des variables d'environnements spécifiés dans les Dockerfile ?

Oui tu peux, ou laisse comme ça.si tu as une bonne raison c'est pas grave

je note ca quelque part ca sera modifié sur la partie mise en prod

worker/worker/redis_interface/challenges.py

worker/worker/redis_interface/details.py

api/api/handlers.py

bonnetn · 2019-09-18T10:56:04Z

api/src/fetch.py

+    data = await redis_app.get(f'{key}')
+    now = datetime.now()
+    timestamp = extract_timestamp_last_update(data)
+    timeout = get_timeout(handler_type)


Pourquoi tu passes pas le timeout directement en argumetn de cette fonction? C'est la fonction parente qui a le plus de connaissance sur quel timeout mettre

api/src/fetch.py

bonnetn · 2019-09-18T11:09:09Z

api/src/fetch.py

+    elif handler_type == 'dynamic_user' and arg is not None and condition:
+        await redis_app.xadd(REDIS_STREAM_USERS, {b'username': arg.encode()})
+    elif handler_type == 'dynamic_categories' and arg is not None and condition:
+        await redis_app.xadd(REDIS_STREAM_CHALLENGES, {b'update': b"ok"})


update:ok est ignoré non? pourquoi mettre une valeur ici?

peu importe oui

worker/worker/__init__.py

worker/worker/redis_interface/challenges.py

bonnetn · 2019-09-18T11:22:15Z

worker/worker/zip.py

@@ -1,4 +1,5 @@
 import itertools
+from typing import List, Tuple


Ce fichier est plus utilisé si?

ca concernera une autre PR ca

bonnetn · 2019-09-18T11:37:17Z

Je pense qu'on peut améliorer la structure du projet:
Pour l'instant il y a deux modules:

redis_interface: qui représente les interactions avec le monde
parser: qui parse

Je trouve que le module parser est super car il est totalement self-contained. Il prend des strings et les parse en structures de données que tu veux, il fait son taff et juste sont taff..

Par contre redis_interface fait un peu tout... Genre dans worker/worker/redis_interface/challenges.py, la fonction retrieve_category_info n'a aucun rapport avec Redis, et en plus elle ne fait pas l'interface avec le système et redis mais plutôt entre le système et Rootme. Alors je sais bien que c'est une fonction qui n'est utilisée que dans ce fichier (d'ailleurs les fonctions non exportées devraient être préfixées d'un _), mais ca correspond quand même pas à ce qu'on attend intuitivement d'une "interface vers redis".

Je pense que l'actuel redis_interface peut être split en 3 modules, ça va t'aider à avoir une architecture plus claire, écrire tes tests plus facilement et potentiellement de pouvoir faire des refactoring plus facilement.
redis_interface pourrait devenir:

redis_interface: garder le module, mais y mettre QUE des fonctions qui font l'interface entre ton système et redis. (en gros ca serait un adapter, dans le "port and adapter pattern")
root_me_interface: Les fonction qui concernent l'interfacage avec rootme, en gros ca serait toujours un appel au HTTP_client + parse et on retourne la valeur. (comme ca ca se teste super simplement)
??? le nom reste à définir (peut être juste worker) qui établit le workflow de ton worker. Genre par exemple, quand on a un ordre d'update, d'abord on uitilise l'interface redis pour récup le timestamp, on check si la resource doit être update, si elle doit être update, on utilise l'interface rootme pour recup les infos, puis on met des données dans redis en utilisant l'interface redis. C'est un exemple de "workflow" qui serait contenu dans ce module

C'est juste une proposition, mais avec ça, juste en lisant la 3ème classe on peut comprendre ce que fait concrètement ton worker, et en lisant les interfaces, on peut comprendre COMMENT il fait

…rom worker

zteeed added 4 commits August 11, 2019 01:44

Add timeloop to worker requirements

2e2350a

Add a timeloop joob to fetch challenges

2b16d69

Add timeloop start to asyncio loop

621a557

Use session requests in http_client script

c11dd63

zteeed requested a review from bonnetn August 11, 2019 02:15

zteeed added 4 commits August 11, 2019 02:33

Use aioredis instead of redis

ccc309f

Add hiredis requirement

dfcbfcf

Manage redis streams for data updates

ffb7ce3

Update constants vars

cf3c955

bonnetn reviewed Aug 11, 2019

View reviewed changes

worker/main.py Outdated Show resolved Hide resolved

zteeed added 3 commits August 11, 2019 15:17

Remove timeloop requirement

6a7cc38

Manage NoneType data in RootMeStaticHandler

c8183a6

Use an external function to fetch data

b9b9da6

zteeed force-pushed the add_redis_stream branch from 7b98ceb to f950f89 Compare September 15, 2019 22:40

zteeed added 3 commits September 16, 2019 00:39

Create consumer groups when api starts

8db3939

Add tasks to update data to streams for worker

ba75cd2

Use redis consumer groups in worker

5116b0b

bonnetn reviewed Sep 16, 2019

View reviewed changes

zteeed force-pushed the add_redis_stream branch from 668a91a to ba8ae70 Compare September 18, 2019 10:44

bonnetn reviewed Sep 18, 2019

View reviewed changes

zteeed added 7 commits September 18, 2019 12:44

Add timestamp data for every task

0b90931

Manage case when user has a null score

fba38a8

Split functions updating data and timestamp for every tasks

22b499a

Send tasks to worker after fetching data if needed

6047a19

Rename consumer group var (more explicit)

6e2dd20

Rename functions for redis consumer group and streams

b2d4a50

Simplify if statement on exception for consumer group creation

4b68d47

zteeed added 11 commits September 18, 2019 12:44

Check redis timestamp data format

689a581

Use class from app object

64eeca9

Use run method from asyncio

005cd70

Integrate timestamp info to redis keys containing data

5a513d9

Unpack list before chaining with itertools

2af4f96

Use typing lib on functions

3689190

Rename function fetching data from redis

3de4c48

Refactor function fetching data from redis using new data structure f…

5bb71a6

…rom worker

Add argument types to functions

3147511

Split dynamic handlers for categories and users data

3fedeed

Split function fetching data and sending tasks to worker

ba8ae70

zteeed merged commit 77fa3cc into master Sep 18, 2019

Use datetime iso format instead of str method

0dca6c5

zteeed deleted the add_redis_stream branch September 25, 2019 03:58

zteeed mentioned this pull request Sep 25, 2019

[ WORKER ] Refactor structure #24

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add redis stream #16

Add redis stream #16

zteeed commented Aug 11, 2019

bonnetn left a comment

bonnetn Sep 16, 2019

zteeed Sep 16, 2019

bonnetn Sep 16, 2019

zteeed Sep 16, 2019

bonnetn Sep 16, 2019

zteeed Sep 18, 2019

bonnetn Sep 18, 2019

bonnetn Sep 18, 2019

zteeed Sep 18, 2019

bonnetn Sep 18, 2019

zteeed Sep 18, 2019

zteeed Sep 18, 2019

bonnetn commented Sep 18, 2019

		@@ -1,4 +1,5 @@
		import itertools
		from typing import List, Tuple

Add redis stream #16

Add redis stream #16

Conversation

zteeed commented Aug 11, 2019

bonnetn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonnetn commented Sep 18, 2019