atlassian-python-api icon indicating copy to clipboard operation
atlassian-python-api copied to clipboard

Return behavior of functions should be normalized

Open kamakazikamikaze opened this issue 6 months ago • 0 comments

The Confluence class has several functions documented to return a specific type. However, when a target page does not exist the function will log an error and return None. This behavior is not always documented for the user to expect/check.

Confluence.get_page_by_title will intercept an error and return None, which is documented for the user.

https://github.com/atlassian-api/atlassian-python-api/blob/63c2faf7decd670bb09de6a6c27d762e9548bd0c/atlassian/confluence.py#L275-L288

https://github.com/atlassian-api/atlassian-python-api/blob/63c2faf7decd670bb09de6a6c27d762e9548bd0c/atlassian/confluence.py#L314-L319

Confluence.get_tables_from_page will return a dictionary with a completely different key structure which isn't documented. Either the same structure should be followed with "number_of_tables_in_page" set to 0 or an exception should be raised.

https://github.com/atlassian-api/atlassian-python-api/blob/63c2faf7decd670bb09de6a6c27d762e9548bd0c/atlassian/confluence.py#L374-L385

Confluence.attach_content will return the API response upon success or None when a target page does not exist. This is not documented.

https://github.com/atlassian-api/atlassian-python-api/blob/63c2faf7decd670bb09de6a6c27d762e9548bd0c/atlassian/confluence.py#L1265-L1268

Confluence.download_attachments_from_page returns a str if no attachments are found, otherwise it returns a dict.

Confluence.update_page will return None if page does not exist. This is not documented for the user.

https://github.com/atlassian-api/atlassian-python-api/blob/63c2faf7decd670bb09de6a6c27d762e9548bd0c/atlassian/confluence.py#L1725-L1728

These are just a small sample of functions where behavior diverges from the documentation or unnecessarily changes the return type. I would suggest either:

  • Raising an exception when a target page does not exist (similar to an ApiNotFoundError, but as a new unique exception). However this would break backwards-compatibility for existing users.
  • Correcting function documentation to warn of the varying return types and/or normalizing structure of certain returns (e.g. dict for Confluence.get_tables_from_page using same keys).

kamakazikamikaze avatar Aug 23 '24 17:08 kamakazikamikaze