Skip to content
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.

Sign up for GitHub

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

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

Merged
merged 9 commits into from
Feb 12, 2024
Merged

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

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
609eadc
feat: consider info if exists rather than the parsed info
Abdellahitech Feb 8, 2024
9dcb6bf
Merge branch 'master' into feat/add-profile-json-to-parsing-warehouse
Abdellahitech Feb 8, 2024
c45593d
fix: add case urls in info of profile json
Abdellahitech Feb 8, 2024
d105ac3
fix: after jamal review
Abdellahitech Feb 10, 2024
eaa09d4
fix: remplace cv url by cv binary (#220)
Abdellahitech Feb 12, 2024
ff11786
fix: correct update date for Jobology connector (#222)
the-forest-tree Feb 12, 2024
6bd7243
4.6.1
Feb 12, 2024
85ba62d
fix: regarding jamal review location=value
Abdellahitech Feb 12, 2024
2218a75
Merge branch 'master' into feat/add-profile-json-to-parsing-warehouse
Abdellahitech Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 49 additions & 3 deletions src/hrflow_connectors/connectors/hrflow/warehouse/profile.py
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ def write_parsing(
api_secret=parameters.api_secret, api_user=parameters.api_user
)
for profile in profiles:
profile_info = profile.pop("info", {})
if parameters.only_insert and hrflow_client.profile.indexing.get(
source_key=parameters.source_key, reference=profile["reference"]
).get("data"):
Expand All @@ -193,7 +194,7 @@ def write_parsing(
)
continue

response = hrflow_client.profile.parsing.add_file(
parsing_response = hrflow_client.profile.parsing.add_file(
source_key=parameters.source_key,
profile_file=profile["resume"]["raw"],
profile_content_type=profile["resume"]["content_type"],
Expand All @@ -202,13 +203,58 @@ def write_parsing(
metadatas=profile["metadatas"],
created_at=profile["created_at"],
)
if response["code"] != 202:
if parsing_response["code"] not in [202, 201]: # 202: Accepted, 201: Created
adapter.error(
"Failed to parse profile with reference={} response={}".format(
profile["reference"], response
profile["reference"], parsing_response
)
)
failed.append(profile)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

source_response = hrflow_client.source.get(
key=parameters.source_key
) # Get source to check if sync_parsing is enabled
if source_response["code"] != 200:
adapter.warning(
"Failed to get source with key={} response={}, won't be able to update"
" profile parsed with profile json".format(
parameters.source_key, source_response
)
)
continue
elif source_response["data"]["sync_parsing"] is True:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

for new_url in profile_info["urls"]:
existing_urls = info_parsed.get("urls", [])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if new_url not in existing_urls:
existing_urls.append(new_url)
info_parsed["urls"] = existing_urls
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

info_parsed[key] = {**info_parsed.get(key, {}), **value}
current_profile["info"] = info_parsed
edit_response = hrflow_client.profile.indexing.edit(
source_key=parameters.source_key,
key=current_profile["key"],
profile_json=current_profile,
)
if edit_response["code"] != 200:
adapter.warning(
"Failed to update profile after parsing, reference={}"
" response={}".format(profile["reference"], edit_response)
)
continue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

return failed


Expand Down
Loading