Skip to content

Conversation

@julienloizelet
Copy link
Collaborator

Fixes #30

@julienloizelet julienloizelet requested a review from rr404 June 21, 2024 05:53
@julienloizelet julienloizelet self-assigned this Jun 21, 2024

@abstractmethod
def get_machine_by_id(self, machine_id: str) -> MachineModel:
def get_machine_by_id(self, machine_id: str) -> Optional[MachineModel]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sql and mongodb storage implementation already returns Optional[MachineModel] ;
None is returned when the machine does not exist in database

)
if exc.response.status_code == 401:
if attempt_count >= self.max_retries:
self.storage.update_or_create_machine(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I delete this piece of code, because we already update the machine in line 165 (after PR, line 157 before PR).

@rr404 : am I missing something ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ça me semble Ok en effet

Copy link
Collaborator

@rr404 rr404 left a comment

Choose a reason for hiding this comment

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

LGTM

total_sent += sent_signal_ids_count
self.logger.info(f"sent {sent_signal_ids_count} signals")
else:
self.logger.error(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Et du coup là on fait rien d'aute que log? sans essayer de lui choper un token ? on prend pour acquis que si ici elle pas a de token c'est qu'il y a eu un aute pb?

Y a t'il un risque qu'elle n'ai pas de token d'ailleurs ? vu que tu hadle le prepare qui fail

)
if exc.response.status_code == 401:
if attempt_count >= self.max_retries:
self.storage.update_or_create_machine(
Copy link
Collaborator

Choose a reason for hiding this comment

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

ça me semble Ok en effet

@julienloizelet julienloizelet merged commit 09b1d48 into crowdsecurity:main Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not block all the sending signals process if something goes wrong with one machine

2 participants