feat: consider info if exists rather than the parsed info #221

Abdellahitech · 2024-02-08T13:20:18Z

No description provided.

the-forest-tree

Merci pour le travail j'ai fini la review.

Je regarde les modifications Jeudi incha Lah. Essaye de faire quelque chose de soigné stp.

the-forest-tree · 2024-02-08T14:02:13Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

            adapter.error(
                "Failed to parse profile with reference={} response={}".format(
-                    profile["reference"], response
+                    profile["reference"], parsing_response
                )
            )
            failed.append(profile)


A priori dans le cas ou ca a failé tu dois rajouter un continue parce que si parsing_response["code"] = 400 par example. Ca va rentrer dans le if puis en sortir et faire le reste c.a.d. la partie hrflow_client.source.get() sauf à la ligne 225 tu fais current_profile = parsing_response["data"]["profile"] mais le cas parsing_response["code"] = 400 tu vas dois rien avoir

the-forest-tree · 2024-02-08T14:05:49Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+            continue
+        elif source_response["data"]["sync_parsing"] is True:


En fait mettre continue suivi d'un elif comme tu fais ici alors que y'a rien après c'est pas vraiment utile. Est ce que tu vois pourquoi ?

A priori soit tu enlèves le continue soit tu changes le elif en if

the-forest-tree · 2024-02-08T14:06:56Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+            else:
+                adapter.warning(
+                    "Failed to get profile after parsing with reference={} response={}"
+                    .format(profile["reference"], parsing_response)
+                )
+                continue


Dans quel cas est ce que ca se passe ca ?

C'est possible que l'API parsing renvoi un 200 ou 201 puis tu as rien dans profile ? ou tu as None ou tu as dict vide ?

the-forest-tree · 2024-02-08T14:09:46Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+            current_profile = parsing_response["data"]["profile"]
+            if current_profile:
+                info_parsed = current_profile.get("info", {})
+                if "urls" in profile_info and isinstance(profile_info["urls"], list):


A mon avis fait juste isinstance(profile_info.get("urls"), list):

the-forest-tree · 2024-02-08T14:10:43Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+                info_parsed = current_profile.get("info", {})
+                if "urls" in profile_info and isinstance(profile_info["urls"], list):
+                    for new_url in profile_info["urls"]:
+                        existing_urls = info_parsed.get("urls", [])


Est ce que ici il n'y a pas le risque que info_parsed.urls ne soit pas une liste ? Parce que tu checks bien avant pour profile_info si c'est une liste ou pas ?

info_parsed.urls vient du parsing donc doit logiquement être une list. Contrairement à profile_info.urls qui vient d'un mapping et on peut faire une erreur sur cela.

the-forest-tree · 2024-02-08T14:20:25Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+                    for new_url in profile_info["urls"]:
+                        existing_urls = info_parsed.get("urls", [])
+                        if new_url not in existing_urls:
+                            existing_urls.append(new_url)
+                        info_parsed["urls"] = existing_urls


Ici quand même le code est compliqué

for new_url in profile_info["urls"]: existing_urls = info_parsed.get("urls", []) <=== Ici pour chaque element `new_url` tu récupères info_parsed.urls if new_url not in existing_urls: existing_urls.append(new_url). <=== eventuellement tu rajoutes info_parsed["urls"] = existing_urls. <==== tu écrases et remet la même liste dans info_parsed.url juste à temps pour la prochaine boucle ou tu récupère encore une fois en début de boucle . Sachant que a priori en plus ca fait rien parce que de base info_parsed["urls"] pointe déjà vers existing_urls

Pourquoi toutes ces complications ?

Pourquoi est ce que tu loops par sur profile_info["urls"] puis tu check if url not in info_parsed["urls"] alors tu rajoutes directement info_parsed["urls"].append(url) ?

Essaye de réecrire le code pour faire plus simple stp

Essaye chez toi en local de faire un truc du genre

a = dict(b=[1, 2, 3]) my_list = a["b"] my_list.append(4) print(a["b"]) # Normalement tu devrais avoir 4 sans avoir a faire avant a["b"] = my_list

the-forest-tree · 2024-02-08T14:30:17Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+                    if value and key != "location" and key != "urls":
+                        info_parsed[key] = value
+                    elif key == "location" and isinstance(value, dict):
+                        if any(value.values()):


Met cette condition après isinstance(value, dict) pour éviter de trop indenter

the-forest-tree · 2024-02-08T14:33:28Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+                    adapter.warning(
+                        "Failed to update profile after parsing, reference={}"
+                        " response={}".format(profile["reference"], edit_response)
+                    )
+                    continue


Regarde si d'un point de vue Business ce cas las ne veut pas dire que c'est une failure ? Si c'est le cas faut rajoute le profile à failed comme ca au moins celui qui utilise l'action sait que pour ce profile la totalité n'a pas marcher.

Mais c'est une decision Produit Management

the-forest-tree · 2024-02-08T14:38:03Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

Si j'ai bien compris le but c'est de garder les information que tu as dans le profile au debut. Puis de mettre ca dans l'input qui est envoyé au call de indexing.edit plutot que faire l'edit avec le retour du Parsing Add.

Dans ce cas tu vas sortir cette logique dans une fonction merge_info(base: dict, info: dict) -> dict
A l'interieur de cette fonction tu vas faire ta logique. Base c'est ce qui vient après Parsing Add, et info c'est le truc auquel tu veux donner plus d'importance

the-forest-tree · 2024-02-12T09:45:30Z

src/hrflow_connectors/connectors/hrflow/warehouse/profile.py

+    for key, value in info.items():
+        if value and key != "location" and key != "urls":
+            info_parsed[key] = value
+        elif key == "location" and isinstance(value, dict) and any(value.values()):


A quoi sert le check any(value.values()) ?

* fix:add some error handling for jobology connector * fix:jobology flake8 connector * fix:some type * fix:regarding jamal review * fix: remplace cv url by cv binary * docs: update docs * fix: flake8 outputs * fix: jobology catch profile * docs: update docs * fix: regarding jamal review * fix: handle possible error binasciii * fix: flake8 and docs * fix: some flake8 output

Co-authored-by: the-forest-tree <the-forest-tree@hrflow.ai>

Automatically generated by python-semantic-release

* feat: consider info if exists rather than the parsed info * fix: add case urls in info of profile json * fix: after jamal review * fix: remplace cv url by cv binary (#220) * fix:add some error handling for jobology connector * fix:jobology flake8 connector * fix:some type * fix:regarding jamal review * fix: remplace cv url by cv binary * docs: update docs * fix: flake8 outputs * fix: jobology catch profile * docs: update docs * fix: regarding jamal review * fix: handle possible error binasciii * fix: flake8 and docs * fix: some flake8 output * fix: correct update date for Jobology connector (#222) Co-authored-by: the-forest-tree <the-forest-tree@hrflow.ai> * 4.6.1 Automatically generated by python-semantic-release * fix: regarding jamal review location=value --------- Co-authored-by: the-forest-tree <65894619+the-forest-tree@users.noreply.github.com> Co-authored-by: the-forest-tree <the-forest-tree@hrflow.ai> Co-authored-by: hrflow-semantic-release <hrflow-semantic-release>

feat: consider info if exists rather than the parsed info

609eadc

Abdellahitech temporarily deployed to ci February 8, 2024 13:20 — with GitHub Actions Inactive

Merge branch 'master' into feat/add-profile-json-to-parsing-warehouse

9dcb6bf

Abdellahitech had a problem deploying to ci February 8, 2024 13:20 — with GitHub Actions Failure

fix: add case urls in info of profile json

c45593d

Abdellahitech temporarily deployed to ci February 8, 2024 13:32 — with GitHub Actions Inactive

Abdellahitech had a problem deploying to ci February 8, 2024 13:35 — with GitHub Actions Failure

the-forest-tree requested changes Feb 8, 2024

View reviewed changes

fix: after jamal review

d105ac3

Abdellahitech temporarily deployed to ci February 10, 2024 16:48 — with GitHub Actions Inactive

Abdellahitech had a problem deploying to ci February 10, 2024 16:53 — with GitHub Actions Failure

the-forest-tree reviewed Feb 12, 2024

View reviewed changes

Abdellahitech and others added 4 commits February 12, 2024 11:14

fix: correct update date for Jobology connector (#222)

ff11786

Co-authored-by: the-forest-tree <the-forest-tree@hrflow.ai>

4.6.1

6bd7243

Automatically generated by python-semantic-release

fix: regarding jamal review location=value

85ba62d

Abdellahitech had a problem deploying to ci February 12, 2024 10:15 — with GitHub Actions Failure

Merge branch 'master' into feat/add-profile-json-to-parsing-warehouse

2218a75

Abdellahitech temporarily deployed to ci February 12, 2024 10:16 — with GitHub Actions Inactive

the-forest-tree merged commit 5b7997b into master Feb 12, 2024
1 check passed

Abdellahitech had a problem deploying to ci February 12, 2024 10:20 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: consider info if exists rather than the parsed info #221

feat: consider info if exists rather than the parsed info #221

Abdellahitech commented Feb 8, 2024

the-forest-tree left a comment

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

Abdellahitech Feb 10, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 8, 2024

the-forest-tree Feb 12, 2024

		continue
		elif source_response["data"]["sync_parsing"] is True:

feat: consider info if exists rather than the parsed info #221

feat: consider info if exists rather than the parsed info #221

Conversation

Abdellahitech commented Feb 8, 2024

the-forest-tree 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