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.

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

Fix/improve entity deletion in QL #275

Closed
Maghnie opened this issue Apr 22, 2024 · 3 comments
Closed

Fix/improve entity deletion in QL #275

Maghnie opened this issue Apr 22, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@Maghnie
Copy link
Contributor

Maghnie commented Apr 22, 2024

  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)
@Maghnie Maghnie added the enhancement New feature or request label Apr 22, 2024
@djs0109
Copy link
Contributor

djs0109 commented Apr 23, 2024

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

Copy link

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

@Maghnie
Copy link
Contributor Author

Maghnie commented May 17, 2024

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

@Maghnie Maghnie closed this as not planned Won't fix, can't repro, duplicate, stale May 17, 2024
@Maghnie Maghnie removed their assignment May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants