RWTH-EBC / FiLiP

FIWARE Library for Python (FiLiP) to work with FIWARE API
BSD 3-Clause "New" or "Revised" License
23 stars 13 forks source link

Fix/improve entity deletion in QL #275

Closed Maghnie closed 3 months ago

Maghnie commented 4 months ago
  1. The indentation of the last three lines in the QL client's delete_entity method makes it seem that it always fails, even after succeeding.

  2. Is the time sleep still needed? It's making the method really slow.

  3. Lastly, after catching the exception, a success message is shown, although you'd expect an error message here.

Suggestion: Adapt the delete_entity method in the QL client to match that in the CB client.

    def delete_entity(self, entity_id: str,
                      entity_type: Optional[str] = None) -> str:
        """
        Given an entity (with type and id), delete all its historical records.

        Args:
            entity_id (String): Entity id is required.
            entity_type (Optional[String]): Entity type if entity_id alone
                can not uniquely define the entity.

        Raises:
            RequestException, if entity was not found
            Exception, if deleting was not successful

        Returns:
            The entity_id of entity that is deleted.
        """
        url = urljoin(self.base_url, f'v2/entities/{entity_id}')
        headers = self.headers.copy()
        if entity_type is not None:
            params = {'type': entity_type}
        else:
            params = {}

        # The deletion does not always resolves in a success even if an ok is
        # returned.
        # Try to delete multiple times with incrementing waits.
        # If the entity is no longer found the methode returns with a success
        # If the deletion attempt fails after the 10th try, raise an
        # Exception: it could not be deleted
        counter = 0
        while counter < 10:
            self.delete(url=url, headers=headers, params=params)
            try:
                self.get_entity_by_id(entity_id=entity_id,
                                      entity_type=entity_type)
            except requests.exceptions.RequestException as err:
                self.logger.info("Entity id '%s' successfully deleted!",
                                 entity_id)
                return entity_id
            time.sleep(counter * 5)
            counter += 1

        msg = f"Could not delete QL entity of id {entity_id}"
        logger.error(msg=msg)
        raise Exception(msg)
djs0109 commented 4 months ago

@Maghnie the implementation is not very intuitive. It works for now, but can be improved

github-actions[bot] commented 4 months ago

Branch 275-Fix/improve-entity-deletion-in-QL created!

Maghnie commented 3 months ago

As discussed offline, since there are other priorities, I'll close this issue for now.