Add workbook to tasks

[williamlang]

According to the docs here:

https://onlinehelp.tableau.com/current/api/rest_api/en-us/help.htm#REST/rest_api_ref.htm#Query_Extract_Refresh_Tasks%3FTocPath%3DAPI%2520Reference%7C_____57

Tasks may or may not have a workbook tag. Added support for this.

[t8y8]

@williamlang is a Tableau employee for those who may not know :)

I'm not super familiar with the Tasks APIs... when does it include a workbook id?

It looks like it could in theory contain a data source as well -- which means we'd want a datasource_id as well OR an 'object_id' and 'object_type' that we handle for the user

I'll take a look at the code in a bit, I reserve the right to propose an alternative ;)

[t8y8]

The code LGTM.

Still thinking about the API...

[williamlang]

@t8y8 you are right though, the second example response does include a datasource id.

[williamlang]

@t8y8 @benlower @RussTheAerialist

Here are a few scenarios of how we could incorporate workbook_id and datasource_id

        # case 1: workbook_id and datasource_id
        workbook_tasks = [[task.id, task.schedule_id, task.workbook_id] for task in all_tasks]
        datasource_taks = [[task.id, task.schedule_id, task.datasource_id] for task in all_tasks]

        for workbook_task in workbook_tasks:
            server.workbooks.get(workbook_task.workbook_id)

        for datasource_task in datasource_taks:
            server.datasources.get(datasource_task.datasource_id)

        # case 2: object_name
        workbook_tasks = filter(lambda task : task.object_name == 'workbook', all_tasks)
        datasource_tasks = filter(lambda task : task.object_name == 'datasource', all_tasks)

        # case 2b: object_name
        for task in all_tasks:
            if task.object_name == 'workbook':
                server.workbooks.get(task.object_id)
            else if task.object_name == 'datasource':
                server.datasources.get(task.object_id)

        # case 3: hash map
        workbook_tasks = filter(lambda task : task.content.type == 'workbook', all_tasks)
        datasource_tasks = filter(lambda task : task.content.type == 'datasource', all_tasks)

        # case 3b: hash_map
        for task in all_tasks:
            if task.content.type == 'workbook':
                server.workbooks.get(task.content.id)
            else if task.content.type == 'datasource':
                server.datasources.get(task.content.id)`

[t8y8]

I think the dict case might be the cleanest API:

for task in all_tasks:
    if task.content['type'] == 'workbook':  # Note that dot notation doesn't work for dicts in python
        print(task.content['id'])

Or

wb_tasks = filter(lambda t: t.content.get('type') == 'workbook', all_tasks)

This is actually a pattern we have to solve for permissions APIs. I slightly prefer the dict but I'm also ok with option 1, simply having a datasource_id AND a workbook_id attribute, and maybe some setter magic to make sure both are never set.

[graysonarts]

Hey @williamlang -

@t8y8 and I talked about the various approaches outlined. I'm not super keen on any of the three as outlined, but I think merging the ideas of 2 and 3 into a 4th option would be the way I'd like to see it happen.

If we had a class called Target with a type and id attribute, then we'd get the ease of use of 3 with the slightly stronger type of 2 without polluting the Task object with prefixed attributes (e.g. task.object.id reads better to me than task.object_id.)

Does that make sense?

[williamlang]

@RussTheAerialist @t8y8

is this what you're thinking?

[t8y8]

Few comments for cleanup.

The question, do we want it to be target_type for the Target class or .type I think .type is fine since it's namespaced in the class.

[t8y8]

Ahh Java snuck into your Python code ;)

+class Target():
    def __init__(self, id, target_type):
        self.id = id
        self.target_type = target_type

    @property
    def target_type(self):
        return self.target_type

Or you can just leave them as attributes without any protection since the object is very simple:

class Target(object):
    def __init__(...):
        self.id = id_
        self.target_type = target_type

    def __repr__(self):
        ...

[t8y8]

Or, if we go with type:

class Target(object):
    def __init__(self, id_, target_type)
        self.id = id_
        self.type = target_type

    @property
    def type(self):
        return self.type

[graysonarts]

I think I'm more of a fan of type rather than target_type because Target is already included in the name of the class, so it ends up being redundant.

[t8y8]

Name the function parameter id_ to avoid collision with the built in.

[t8y8]

Two blank lines here (between docstring and the class def) should fix the style checker
tableauserverclient/models/target.py:2:1: E302 expected 2 blank lines, found 0

[graysonarts]

I think you'll need a .format(**self) at the end of the repr.

[graysonarts]

Why was this line removed?

[williamlang]

@RussTheAerialist added a test to prevent schedule_id getting deleted again :)

@t8y8 did the thing.

[t8y8]

🚀

Two small nits

[t8y8]

Remove this extra '#' and the space around it (of course still making the style checker happy)

[t8y8]

This is technically never possible, right? (though a test for it is fine -- we may do this someday)

[t8y8]

🚀🚀

[graysonarts]

🚀 👨‍🎤