Skip to content

public api for collaboration features#22

Merged
spicquenot merged 18 commits intorelease/5.0from
feature/collaboration
Jul 9, 2018
Merged

public api for collaboration features#22
spicquenot merged 18 commits intorelease/5.0from
feature/collaboration

Conversation

@spicquenot
Copy link
Contributor

@spicquenot spicquenot commented Feb 2, 2018

Adding public api for collaboration features:

  • Wiki (almost done, need to wait for attached files)
  • Discussions

@datastrid
Copy link
Contributor

I'll need this in master to run the wiki tests on Jenkins 😢

@spicquenot
Copy link
Contributor Author

@datastrid I have added upload for a file attachment of an article, can you check it works ok before merging (and forgetting about the poor home pages :p )

@datastrid
Copy link
Contributor

@spicquenot the file upload works but it doesn't handle encoding right ("héhéhé.pdf" gives "heÌ�heÌ�heÌ�.pdf" in dss) ❌
(when downloading the file, it gets its encoding back)

@spicquenot
Copy link
Contributor Author

@datastrid
Copy link
Contributor

@spicquenot 👍

@cstenac
Copy link
Contributor

cstenac commented Jul 2, 2018

Sorry but this does not work. Let's talk to rewrite this API using our newer design elements, and to provide useful helpers.

@cstenac
Copy link
Contributor

cstenac commented Jul 2, 2018

Proposed draft for wiki API:


class DSSWiki(object):
    """
    A handle to manage the wiki of a project
    """
    def __init__(self, client, project_key):
        self.client = client
        self.project_key = project_key

    def get_settings(self):
        Perform the call to get here
        Return a DSSWikiSettings

    def list_articles(self):
        Perform a call to get settings
        Return flattened list of DSSWikiArticle. Much more directly usable than the taxonomy

    def get_article(self, article_id):
        Perform call to get this article
        return DSSWikiArticle(self, article_id)

    def create_article(self, article_id, parent_id, initial_body=""):
        """
        --> Useful to be able to set the body here without having to do a second back and forth

        Create a wiki article

        Args:
            the article ID
            the parent article ID or None if no parent (at wiki root scope)

        Returns --> a DSSWikiArticle !
        """
        body = {
            "projectKey": self.project_key,
            "id": article_id,
            "parent": parent_id
        }
        self.client._perform_json("POST", "/projects/%s/wiki/" % (self.project_key), body=body)
        return DSSWikiArticle....

class DSSWikiSettings(object):
    """Global settings of the wiki, including the taxonomy. Call save() to save"""

    def __init__(settings):
        self.settings = settings

    def get_taxonomy(self):
        return self.settings["taxonomy"]

    def set_taxonomy(self):
        Set in settings

    # Helper to edit the taxonomy
    def set_article_as_child_of(self, article_id, new_parent_id):
        TODO

    def get_home_article():
        return from settings

    def set_home_article():
        Set in settings

    def save():
        # Perform the call to save


class DSSWikiArticle(object):
    """
    A handle to manage an article.
    """
    def __init__(self, client, project_key, article_id):
        self.client = client
        self.project_key = project_key
        self.article_id = article_id
        ##### VERY VERY VERY VERY SUSPICIOUS !!! PROBABLY NOT PYTHON3 SAFE !! #######
        if isinstance(self.article_id, unicode):
            self.article_id = self.article_id.encode('utf-8')

    def get_data(self):
        Perform call to Get
        return DSSWikiArticleData()

    def upload_attachment(self, fp):
        """
        Upload an attachment file and attaches it to the article

        Args:
            A file-like object that represents the upload file

        Return nothing
        """
        return self.client._perform_json("POST", "/projects/%s/wiki/%s/upload" % (self.project_key, urllib.quote(self.article_id)), files={"file":fp})

    def delete(self):
        """
        Delete the article
        """
        self.client._perform_empty("DELETE", "/projects/%s/wiki/%s" % (self.project_key, urllib.quote(self.article_id)))

    ########################################################
    # Discussions
    ########################################################
    def get_object_discussions(self):
        """
        Get a handle to manage discussions on the article

        Returns:
            the DSSObjectDiscussions of this article
        """
        return DSSObjectDiscussions(self.client, self.project_key, "ARTICLE", self.article_id)


class DSSWikiArticleData(object):
    """Metadata and body of a wiki article.  You must call save() if you want to make changes to this"""
    def __init__(self, client, project_key, article_id, article_data):
        self.client = client
        self.project_key = project_key
        self.article_id = article_id
        ##### VERY VERY VERY VERY SUSPICIOUS !!! PROBABLY NOT PYTHON3 SAFE !! #######
        if isinstance(self.article_id, unicode):
            self.article_id = self.article_id.encode('utf-8')

        self.article_data = article_data

    def get_body(self):
        """Gets the Markdown body as a string"""
        return self.article_data["payload"]

    def set_body(self, new_body):
        """Update the Markdown body. new_body is a string"""
        self.article_data["payload"] = new_body

    def get_metadata(self):
        """Get metadata as a dict"""
        return self.article_data["article"]

    def set_metadata(self, new_meta):
        """new_meta is a dict"""
        self.article_data["article"] = new_meta

    def save(self):
        # Perform API call to save here

@cstenac
Copy link
Contributor

cstenac commented Jul 2, 2018

Proposal for discussion API:

import json

class DSSObjectDiscussions(object):
    """
    A handle to manage discussions on a DSS object
    """
    def __init__(self, client, project_key, object_type, object_id):
        self.client = client
        self.project_key = project_key
        self.object_type = object_type
        self.object_id = object_id

    ##########################
    # Discussions on an object
    ##########################
    def list_discussions(self):
        """
        Get list of discussions

        Returns: a list of DSSDiscussion preseeded wiht discussion_data_has_replies = False
        """
        list = self.client._perform_json("GET", "/projects/%s/discussions/%s/%s/" % (self.project_key, self.object_type, self.object_id))
        ....

    def create_discussion(self, topic, message):
        """
        Create a discussion
        Returns:
            The newly created discussion
        """
        d = self.client._perform_json("POST", "/projects/%s/discussions/%s/%s/" % (self.project_key, self.object_type, self.object_id), body=discussion_creator.get_data())
        return DSSDiscussion(d)

    def get_discussion(self, discussion_id):
        """
        Get a specific discussion

        Returns:
            a discussion
        """
        return DSSDiscussion(self.client, self.project_key, self.object_type, self.object_id, discussion_id)


class DSSDiscussion(object):
    """
    A handle to manage a single discussion
    """
    def __init__(self, client, project_key, object_type, object_id, discussion_id, discussion_data, discussion_data_has_replies):
        self.client = client
        self.project_key = project_key
        self.object_type = object_type
        self.object_id = object_id
        self.discussion_id = discussion_id

    def _get_with_replies():
        # Perform the call, update self.discussion_data and self.discussion_data_has_replies

    def get_metadata():
        # Get from self.discussion_data

    def save_metadata():
        # update discussion_data and save directly
        # small deviation from what we do usually for simplicity$
        # (ie no explicit save call)

    def get_messages():
        if not self.discussion_data_has_replies:
            self._get_with_replies()
        # Returns an array of DSSDiscussionMesssage

    def add_reply(self, text):
        """
        Add a reply to a discussion

        Args:
            The reply message text

        Returns: nothing ! Updates the discussion_data object
        """
        reply_data = {
            "reply": reply
        }
        
        self.client._perform_json("POST", "/projects/%s/discussions/%s/%s/%s/replies/" % (self.project_key, self.object_type, self.object_id, self.discussion_id), body=reply_data)
        self.discussion_data = ...



class DSSDiscussionMessage():
    """Read only accessor"""
    def __init__(self, raw):
         ...

    def get_raw():
        return self.raw

    def get_text():
        return self.raw["text"]

    def get_author():
        pass

    def get_time():
        pass

@cstenac
Copy link
Contributor

cstenac commented Jul 2, 2018

Also: all methods in PublicAPIDiscussionsController uses checkPerm(req) which only works with UI users, so it just doesn't work with generic API keys.

You need to use authCtx = getTicketOrKey() and checkPerm(authCtx)

@cstenac
Copy link
Contributor

cstenac commented Jul 2, 2018

Generic things: use newer-style comments as found in ml.py, document all return types

@datastrid
Copy link
Contributor

@cstenac what is not working? did I test badly? did an issue come back from hell?

@spicquenot spicquenot changed the base branch from master to release/5.0 July 2, 2018 12:20
@spicquenot
Copy link
Contributor Author

Should be more or less okay

@cstenac cstenac self-requested a review July 9, 2018 08:50
Copy link
Contributor

@cstenac cstenac left a comment

Choose a reason for hiding this comment

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

Minor nitpick

"""
data = self.client._perform_json("GET", "/projects/%s/discussions/%s/%s/" % (self.project_key, self.object_type, self.object_id))

discu_list = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: pythonic way would be a single line

return [DSSDiscussion(x.blablabla) for x in data]


class DSSObjectDiscussions(object):
"""
A handle to manage discussions on a DSS object
Copy link
Contributor

Choose a reason for hiding this comment

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

Indicate that users should not create this object themselves and how to create it (same thing for all objects of this PR)

For example, we have that in ML:

"""Do not call directly, use :meth:`DSSMLTaskSettings.get_split_params`"""

"""
self.reply_data = reply_data

def get_reply_data(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably include "raw" in the name to make it clear that it's "hardcore" stuff. In many cases, we just use "get_raw". "get_raw_data" would work too. Having "reply" in the name is generally not required for methods in a class called "reply"

Get the wiki

:returns: the wiki associated to the project
:rtype: DSSWiki
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, this should be a link to the class

:rtype: :class:`dataiku.dss.DSSWiki`

"""
Get wiki settings

:returns: an handle to manage the wiki settings (taxonomy, home article)
Copy link
Contributor

Choose a reason for hiding this comment

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

a :)


def list_articles(self):
"""
Get a list of all the articles
Copy link
Contributor

Choose a reason for hiding this comment

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

should mention that each itme in the returned list will be a DSSWikiArticle (with link to the class)

Get a list of all the articles

:returns: list of articles
:rtype: list
Copy link
Contributor

Choose a reason for hiding this comment

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

same: you can use "list of :class:`...."


def get_taxonomy(self):
"""
Get the taxonomy
Copy link
Contributor

Choose a reason for hiding this comment

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

This would require a bit more documentation about what kind of things are in the taxonomy. It seems that the helper "move article in taxonomy" is missing :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also need to explain that you get a direct reference, not a copy, so modifications to the returned object will be reflected when saving

article_data = self.client._perform_json("GET", "/projects/%s/wiki/%s" % (self.project_key, dku_quote_fn(self.article_id)))
return DSSWikiArticleData(self.client, self.project_key, self.article_id, article_data)

def upload_attachement(self, fp):
Copy link
Contributor

Choose a reason for hiding this comment

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

Add filename

@spicquenot spicquenot merged commit 2ee3d75 into release/5.0 Jul 9, 2018
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.

3 participants