Adding Schedules Support Round 2 #48

t8y8 · 2016-10-03T21:11:02Z

Resubmitted @shinchris 's PR #30 with a few small changes -- I updated the old PR with comments on my own review.

…t-python into 43-schedules-support

t8y8 · 2016-10-04T16:34:22Z

tableauserverclient/models/schedule_item.py

+        return self._execution_order
+
+    @execution_order.setter
+    def execution_order(self, value):


Need to update this with @LGraber 's decorators still

graysonarts

Neither of my comments are required, as is, this is good 🚀

graysonarts · 2016-10-04T17:01:19Z

tableauserverclient/models/schedule_item.py

+
+    def _set_values(self, id, name, state, created_at, updated_at, schedule_type, frequency,
+                    next_run_at, end_schedule_at, execution_order, priority, interval_item):
+        if id is not None:


I feel like accepting **kwargs and then having a list of accepted attributes and iterating over them might be cleaner than all of these.

# Assume some static value called "ALLOWED_ATTRIBUTES" def _set_values(self, **kwargs): for attr_name in self.ALLLOWED_ATTRIBUTES: if attr_name in kwargs and kwargs[attr_name]: setattr(self, '_{}'.format(attr_name), kwargs[attr_name]

Since this is mostly a helper method, I think it's okay to use the kwargs

I'm on board -- there are several items where we should do that (site_item is also hilariously large). I'll do that in another PR -- I'll pick one and try it, then apply it to all of them

graysonarts · 2016-10-04T17:04:00Z

tableauserverclient/server/request_factory.py

+    def update_req(self, schedule_item):
+        xml_request = ET.Element('tsRequest')
+        schedule_element = ET.SubElement(xml_request, 'schedule')
+        if schedule_item.name:


This could also be turned in a loop across all the different attributes. since it's pretty prescriptive, the only tricky-ish part is the conversion of things like executionOrder to execution_order, but I think that's well understood.

t8y8 · 2016-10-04T17:59:06Z

tableauserverclient/models/schedule_item.py

@@ -1,5 +1,6 @@
 import xml.etree.ElementTree as ET
 from .interval_item import IntervalItem
+from .property_decorators import property_is_enum, property_is_boolean, property_not_empty, property_not_nullable


I will remove the imports I'm not using, copy/paste error

graysonarts

🚀 man burning out his fuse up here alone

LGraber · 2016-10-04T19:43:15Z

I would like to talk about this. I have a lot of trouble following the "IntervalItem" logic. It seems like we are trying to force 4 classes into one and the meaning of various fields changes based on one field. Given the complexity that ends up being introduced, I guess I don't see why we didn't just create appropriate classes for the different types of intervals. The result of the above code is that after I create an item, I can't change it. I have to recreate one. If we think of how a user flow might work here, that does not feel like a good experience. Lets talk about this.

t8y8 · 2016-10-04T19:57:46Z

I don't think having to recreate an IntervalItem is a big deal -- these are basically thin wrappers over combining some enums together into a usable blob to pass to a schedule. I don't think it's going to be common to reuse an interval instance.

Actually, this follows the stdlib pattern for datetime pretty closely.

important_schedule = IntervalItem.create_hourly(start, end, occurrence, value)
less_important_schedule = IntervalItem.create_daily(start_time)

for extract in extracts:
    if extract_is_important(extract):
        extract.update_schedule(important_schedule)
    else:
        extract.update_schedule(less_important_schedule)

I do agree that the class itself is pretty large and hairy and could be broken down into smaller classes for readability.

The above code could then be something like:

important_schedule = HourlyInterval(start, end, occurrence, value)
less_important_schedule = DailyInterval(start_time)

for extract in extracts:
    if extract_is_important(extract):
        extract.update_schedule(important_schedule)
    else:
        extract.update_schedule(less_important_schedule)

A little nicer, but feels about the same -- I think the bigger wins are for code maintainability.

I'm guessing Chris was just following the patterns of similar our APIs (with the nested enum classes) and staying a fairly thin wrapper over our XML schema. Admittedly this is also the most complicated type in our schema :/

We can chat about your thoughts to clean up the class next time I'm in the building! (Or via email/hipchat, whatever)

EDIT: I realize that keeping this as one class makes it easier to pull it from an XML response, otherwise we'd have to do a little more work

graysonarts · 2016-10-04T20:49:34Z

From my viewpoint, the second code block that you provided is better. 4 separate classes is more clear to the user, and more clear on being able to make modifications to the schedule/interval.

RE: easier to parse the xml vs. easier for the user to grok. I'm going to go with easier for the user to grok 100% of the time. The little extra work we need to do in parsing out to different classes is minimal and worth the effort for clarity

t8y8 · 2016-10-05T01:57:27Z

@LGraber @RussTheAerialist here's a first stab at it -- there's a lot of validation logic I haven't ported to the individual Interval classes yet...

But tests pass!

t8y8 · 2016-10-05T03:01:27Z

tableauserverclient/models/interval_item.py

@@ -11,8 +11,8 @@ class Frequency:
        Monthly = "Monthly"

    class Occurrence:
-        Hours = "Hours"
-        Minutes = "Minutes"
+        Hours = "hours"


The XSD actually specifies these as lower case, I'm not sure how it was working before to be honest.

But I found these by getting test failures after the refactor

t8y8 · 2016-10-05T03:01:56Z

tableauserverclient/models/interval_item.py

+                interval_occurrence != IntervalItem.Occurrence.Minutes:
+            error = "Invalid interval type defined: {}.".format(interval_occurrence)
+            raise ValueError(error)
+        elif interval_occurrence == IntervalItem.Occurrence.Hours and int(interval_value) not in [1, 2, 4, 6, 8, 12]:


Similarly I'm not sure how this worked before, parsed responses will be strings, found via test failures.

t8y8 · 2016-10-05T03:02:25Z

tableauserverclient/models/interval_item.py

@@ -102,44 +117,10 @@ class MonthlyInterval(IntervalItem):
    def __init__(self, start_time, interval_value):
        self._validate_time(start_time)

+        if (int(interval_value) < 1 or int(interval_value) > 31) and interval_value != IntervalItem.Day.LastDay:


Again, not sure how this worked without converting to an int before

LGraber · 2016-10-05T04:21:45Z

Are you reviewing 'your' changes now?

t8y8 · 2016-10-05T04:24:19Z

Just leaving comments for you guys :) -- it's not 100% ready yet but wanted it up for eyes on it

LGraber · 2016-10-05T04:24:30Z

It looks like you are mid change now. I am expecting their will be 4 "Interval" classes which can be modified after creation with validation. You cannot change hourly to monthly by simply changing a field but that is fine. They are different classes with very different settings. The IntervalItem class can go away. Let me know when I should take a look again or if you want me to take a stab.

t8y8 · 2016-10-05T17:36:56Z

@LGraber I still don't think that use case will be common. People aren't going to be likely to create morhping_interval and constantly update it.

every_tuesday = WeeklyInterval(start_time, Interval.Occurance.Day.Tuesday)
awesome_schedule = Schedule(name='The Best', interval=every_tuesday, *args)
for wb in wbs:
    add_to_schedule(wb, awesome_schedule)

It's easy enough to attach setters that trigger validation logic so you can do that if we really need to -- but I'm not sure it's worth it at all. (@benlower, any use cases come to mind?)

For the breakdown of the classes: I've got a question

As currently implemented I have the Daily/Weekly/Hourly/Monthly Intervals inheriting from IntervalItem. IntervalItem owns the parsing logic and building the more specific IntervalItem types, as well as holding their shared enums and default instance variables (start_time, end_time, frequency).

It also lets us keep the parsing logic centralized in IntervalItem which acts as a proxy and generates the more specific IntervalType. I liked not having to change all the schedule_endpoints so I kept it, but if there's a suggestion for an alternative I'm all ears (/cc @RussTheAerialist )

LGraber · 2016-10-05T18:55:06Z

The scenario is that I query the rest endpoint to get the schedule and then I want to change it.

t8y8 · 2016-10-05T19:56:49Z

I think the use case is well served by just creating a new, clean, Interval and adding it to the schedule you're updating -- but I can add the getters/setters pretty easily, I'll whip it up todayish.

As a bonus, it does move the validation logic out of the initializer

LGraber · 2016-10-05T19:59:17Z

I think you are going to like what the code looks like.

t8y8 · 2016-10-05T20:54:22Z

@LGraber Ok fine, maybe it looks a little cleaner.

Maybe 💛

t8y8 · 2016-10-05T21:37:51Z

samples/create_schedules.py

+    server = TSC.Server(args.server)
+    with server.auth.sign_in(tableau_auth):
+        # Hourly Schedule
+        hourly_interval = TSC.IntervalItem.create_hourly(time(2, 30), time(23, 0), TSC.IntervalItem.Occurrence.Hours, 2)


Yikes, I need to update these now :)

(I wonder if we can find a way to run these, either in a local task, or in travis somehow)

…for various interval constants

t8y8 · 2016-10-06T04:34:46Z

tableauserverclient/server/endpoint/schedules_endpoint.py

+        update_req = RequestFactory.Schedule.update_req(schedule_item)
+        server_response = self.put_request(url, update_req)
+        logger.info("Updated schedule item (ID: {})".format(schedule_item.id))
+        updated_schedule = copy.copy(schedule_item)


@LGraber or @RussTheAerialist any idea why we're doing a copy here? It seems like we can just instantiate a ScheduleItem from the response just like create and avoid copy.copy which is well... unpythonic ;) (but also unnecessary)

url = "{0}/{1}".format(self.baseurl, schedule_item.id) update_req = RequestFactory.Schedule.update_req(schedule_item) server_response = self.put_request(url, update_req) logger.info("Updated schedule item (ID: {})".format(schedule_item.id)) updated_schedule = ScheduleItem.from_response(server_response.content)[0] return updated_schedule

It also let's me kill ScheduleItem._parse_common_tags which is used only for this dance.

EDIT: All *_item classes have that method, _parse_common_tags that seems to get used only in update calls, but I'm pretty sure we can just do ItemClass.from_response() for all of those

graysonarts · 2016-10-06T05:06:19Z

Iirc, There are some properties that aren't returned on the update that are
returned on the query, so we create a copy and update the new copy we
return. This also keeps in line with the "we don't modify existing objects
on a result, but return a new copy"
On Wed, Oct 5, 2016 at 21:34 Tyler Doyle notifications@github.com wrote:

@t8y8 commented on this pull request.

In tableauserverclient/server/endpoint/schedules_endpoint.py
#48 (review)
:
   self.delete_request(url)
   logger.info("Deleted single schedule (ID: {0})".format(schedule_id))
def update(self, schedule_item):
   if not schedule_item.id:
       error = "Schedule item missing ID."
       raise MissingRequiredFieldError(error)
   if schedule_item.interval_item is None:
       error = "Interval item must be defined."
       raise MissingRequiredFieldError(error)
   url = "{0}/{1}".format(self._construct_url(), schedule_item.id)
   update_req = RequestFactory.Schedule.update_req(schedule_item)
   server_response = self.put_request(url, update_req)
   logger.info("Updated schedule item (ID: {})".format(schedule_item.id))
   updated_schedule = copy.copy(schedule_item)
@LGraber https://github.com/LGraber or @RussTheAerialist
https://github.com/RussTheAerialist any idea why we're doing a copy
here? It seems like we can just parse the response from the Update request
and avoid copy.copy which is well... unpythonic ;) (but also unnecessary)
    url = "{0}/{1}".format(self.baseurl, schedule_item.id)
    update_req = RequestFactory.Schedule.update_req(schedule_item)
    server_response = self.put_request(url, update_req)
    logger.info("Updated schedule item (ID: {})".format(schedule_item.id))
    updated_schedule = ScheduleItem.from_response(server_response.content)[0]
    return updated_schedule
It also let's me kill ScheduleItem._parse_common_tags which is used only
for this dance

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#48 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFxVRBf6AjQ5E5Us-mWF1K8zy5oHHZZks5qxHpmgaJpZM4KNDAW
.

LGraber · 2016-10-06T04:39:05Z

samples/create_schedules.py

+    server = TSC.Server(args.server)
+    with server.auth.sign_in(tableau_auth):
+        # Hourly Schedule
+        hourly_interval = TSC.HourlyInterval(time(2, 30), time(23, 0), TSC.IntervalItem.Occurrence.Hours, 2)


Can we expand the comment above to atleast indicate what this schedule is. Setting everything in the constructor makes it really hard to figure out what this is actually doing.

Done. Added keyword arguments to several of them as well so the values are clearer

LGraber · 2016-10-06T04:39:59Z

samples/create_schedules.py

+        print("Hourly schedule created (ID: {}).".format(hourly_schedule.id))
+
+        # Daily Schedule
+        daily_interval = TSC.DailyInterval(time(5))


Again, add a comment to explain what this means

LGraber · 2016-10-06T04:40:50Z

samples/create_schedules.py

+        print("Daily schedule created (ID: {}).".format(daily_schedule.id))
+
+        # Weekly Schedule
+        weekly_interval = TSC.WeeklyInterval(time(19, 15), TSC.IntervalItem.Day.Monday,


LGraber · 2016-10-06T04:41:12Z

samples/create_schedules.py

+        print("Weekly schedule created (ID: {}).".format(weekly_schedule.id))
+
+        # Monthly Schedule
+        monthly_interval = TSC.MonthlyInterval(time(23, 30), 15)


LGraber · 2016-10-06T04:41:49Z

tableauserverclient/__init__.py

    SiteItem, TableauAuth, UserItem, ViewItem, WorkbookItem, UnpopulatedPropertyError
 from .server import RequestOptions, Filter, Sort, Server, ServerResponseError,\
    MissingRequiredFieldError, NotSignedInError
+from .models.interval_item import *


Russell didn't like it when I imported *

aww, but I'm lazy. ok done.

LGraber · 2016-10-06T05:11:10Z

tableauserverclient/models/schedule_item.py

+        self.schedule_type = schedule_type
+
+    @property
+    def created_at(self):


This has to be a datetime, right?

We currently parse it as a string from the XML -- it's read only so I think that's fine

LGraber · 2016-10-06T05:13:06Z

tableauserverclient/models/schedule_item.py

+        self._created_at = None
+        self._end_schedule_at = None
+        self._execution_order = None
+        self._frequency = None


what is frequency if I have an interval associated with this? That object contains a frequency. What is this? Does this just reference the "frequency" in the interval? If so, why do we have an actual property here instead of just accessing the frequency member of the Interval?

You'll need to take a look at the XSD (and grab a glass of wine). On the schedule level there's a frequency attribute (Daily, Hourly, Monthly) and then the interval is a frequenyDetail

We still shouldn't let someone set this and it needs to somehow be tied to the actual interval. As it is, it seems like I could change the "interval" and not this and we would be weirdly broken. The server design seems weird but we should block mistakes because of it.

There isn't a setter for frequency on scheduleitem. It appears to only get set when serializing/deserializing based on the intervalitem. I think.

Either way, I don't think a user can break it unless they set it directly with blah._freq = 'BadFreq' -- which is ok, that's a violation of the norm

What happens if they get a schedule from the server, change the interval from Hourly to Monthly. The "frequency" getter is wrong and won't our serialization be wrong if the user tries to update the server?

I don't think so, can you reproduce that?

The serializer always sets it off the interval_item frequency:

interval_item = schedule_item.interval_item if interval_item._frequency: schedule_element.attrib['frequency'] = interval_item._frequency frequency_element = ET.SubElement(schedule_element, 'frequencyDetails') frequency_element.attrib['start'] = str(interval_item.start_time) if interval_item.end_time: frequency_element.attrib['end'] = str(interval_item.end_time)

In fact, the only place frequency appears to be used at all is in the parser to determine which IntervalItem to select... I can probably just remove it from the class entirely

I just confirmed I can remove that attribute entirely, tests pass (other than the ones that check for it explicitly), and the sample still runs... should I nix it entirely?

@LGraber @RussTheAerialist

LGraber · 2016-10-06T05:13:35Z

tableauserverclient/models/schedule_item.py

+        self._execution_order = value
+
+    @property
+    def frequency(self):


I think this is supposed to access the interval's frequency

Nope, the frequency is on the schedule, the interval is the frequencyDetail

http://onlinehelp.tableau.com/samples/en-us/rest_api/ts-api_2_3.xsd

LGraber · 2016-10-06T05:14:54Z

tableauserverclient/models/schedule_item.py

+
+    @priority.setter
+    def priority(self, value):
+        if value < 1 or value > 100:


feels like "@property_is_int(min,max)" to be consistent with our other handling. min = None and/or max = None meaning don't check that boundary

Working on this now, the min or max=None is a little tricky.

Ok, done.

def property_is_int(min, max=2**31): def property_type_decorator(func): @wraps(func) def wrapper(self, value): if min is None: return func(self, value) if value < min or value > max: error = "Invalid priority defined: {}.".format(value) raise ValueError(error) return func(self, value) return wrapper return property_type_decorator

This means that passing just None will skip validation, otherwise it's min/max.

Technically to make this signature work, Max has a default value of 2**31

LGraber · 2016-10-06T05:18:42Z

tableauserverclient/server/endpoint/schedules_endpoint.py

+        self.baseurl = "{0}/schedules"
+        self.parent_srv = parent_srv
+
+    def _construct_url(self):


We changed the pattern here so that it was in sync with the root server class. We don't use _construct_url. We just made base_url a property and build the string when you call it. Let's be consistent. It was done after Chris wrote the original pass on this but we should update.

Yup, noticed this too, I've got this queued up for the next rev

t8y8 · 2016-10-06T22:43:57Z

tableauserverclient/models/schedule_item.py

@@ -0,0 +1,236 @@
+import xml.etree.ElementTree as ET
+from .interval_item import *


I'll fix this to not be a *

LGraber · 2016-10-07T04:36:41Z

tableauserverclient/models/interval_item.py

+
+    class Occurrence:
+        Hours = "hours"
+        Minutes = "minutes"


Minutes should come before hours.

LGraber · 2016-10-07T06:30:00Z

tableauserverclient/models/interval_item.py

+        else:
+            interval_value = interval
+            interval_occurrence = IntervalItem.Occurrence.Hours
+
        self._interval = [(interval_occurrence, str(interval_value))]


How do I read the interval? I expect to be able to read the value I set or one that was returned from a "get" call to the server.

Intervals are tuples of (Occurence, Value), so in this case it would return

('hours', 5) or ('minutes', 15) or ('weekDay', 'Monday')

It's consistent across the different interval values -- which is how the serializer wants it:

if interval_item.interval: intervals_element = ET.SubElement(frequency_element, 'intervals') for interval in interval_item.interval: expression, value = interval single_interval_element = ET.SubElement(intervals_element, 'interval') single_interval_element.attrib[expression] = value

I want to return what we set. We have made the "Occurence" disappear. Why are we putting it back? if I write:
foo.interval = .25
and then do
x = foo.interval

x should equal .25

We can make the serialization work without exposing this to users.

I think that's going to be a fairly large change.

We'd need a serializer for each IntervalType maybe?

Or... I'd have to think more about it, when it's not so late.

We could do it in the serializer or you can have an internal function on each of them which returns this pair. We can call internal functions from our serializer ... this is already a pattern we use. But the value I set for interval should be the value I get back for it (and I should be able to directly set it, ie not just in the constructor).

I like the latter

interval_item.interval_pairs() or whatever.

One-line change in the serializer, and then the internal function is easy to write as well.
I'll give it a go tomorrow.

LGraber · 2016-10-07T06:34:49Z

tableauserverclient/models/interval_item.py

+    def __init__(self, start_time):
+        self.start_time = start_time
+        self._frequency = IntervalItem.Frequency.Daily
+        self.end_time = None


why do you have this (besides the issue with serialization)? I would rather have the serializer clean it than have properties which are meaningless but visible.

Are you talking about the end_time arritbutes?

Yeah, that was so I didn't have to re-write the serializer, which I consider to be the riskiest part.

We'd have to add a bunch of hasattr or similar checks into the serializer, I think it's the lesser of two evils to have the empty attributes here

LGraber · 2016-10-07T06:34:59Z

tableauserverclient/models/interval_item.py

+        self.start_time = start_time
+        self._frequency = IntervalItem.Frequency.Daily
+        self.end_time = None
+        self.interval = None


Same for this

LGraber · 2016-10-07T06:35:15Z

tableauserverclient/models/interval_item.py

+        self.start_time = start_time
+        self._frequency = IntervalItem.Frequency.Weekly
+        self.interval = interval_values
+        self.end_time = None


LGraber · 2016-10-07T06:35:43Z

tableauserverclient/models/interval_item.py

+        self.start_time = start_time
+        self._frequency = IntervalItem.Frequency.Monthly
+        self.interval = str(interval_value)
+        self.end_time = None


And here. No?

Done, you're right, it doesn't look that bad

LGraber · 2016-10-07T06:38:07Z

tableauserverclient/models/property_decorators.py

+        @wraps(func)
+        def wrapper(self, value):
+            if min is None:
+                return func(self, value)


I don't get this? Why can't I set the min to None and the max to some number (like 100). Not saying you are doing this right now but this code would break. Just do the if "min is not None and value < min" and similarly "if max is not None and value > max". No?

I could enable that, but then how would we say "this should be an int but not checked within a range"

property_is_int(None) is nice and clean to say that.
property_is_int(None, 100) is ambiguous with the above call because both min and max are required positional arguments, so no nice signature :(

I could:

Make a keyword argument that is property_is_int(range=False) or some similar keyword.

I am fine with accepting a pair using range=(None,100). It is more readable. I am good with that.

Done.

def property_is_int(range): def property_type_decorator(func): @wraps(func) def wrapper(self, value): error = "Invalid priority defined: {}.".format(value) if range is None: if isinstance(value, int): return func(self, value) else: raise ValueError(error) min, max = range if value < min or value > max: raise ValueError(error) return func(self, value) return wrapper return property_type_decorator

property_is_int(range=None) means "int but it can be whatevs"
property_is_int(range=(1,100)) means "int between 1 and 100"

Okay. I am fine with this. If it is ever actually a need, we can debate a case with only a max or only a min but we have no case so I am not going to debate just for the sake of debating. I like the syntax. Thanks!

LGraber · 2016-10-07T06:40:28Z

tableauserverclient/server/request_factory.py

+        schedule_element.attrib['frequency'] = interval_item._frequency
+        frequency_element = ET.SubElement(schedule_element, 'frequencyDetails')
+        frequency_element.attrib['start'] = str(interval_item.start_time)
+        if interval_item.end_time:


add hasattr(interval_item, end_time)

LGraber · 2016-10-07T06:43:22Z

I gave 4-5 more comments but they should be small and easy fixes. Then I will be ready to signoff. There might be more to cleanup but I really like the .5 and .25 as it just really cleans up some of the code and makes this a bit more sensible for me. Thanks!

…serializer and keep the interval round-trippable now for all interval types. Minor cleanup. Note there's still casting between types because of how we parse from the XML, I will clean that up in another PR

LGraber · 2016-10-07T18:41:01Z

tableauserverclient/server/request_factory.py

-                single_interval_element = ET.SubElement(intervals_element, 'interval')
-                single_interval_element.attrib[expression] = value
+        if hasattr(interval_item, 'end_time'):
+            if interval_item.end_time:


you could put this on the hasattr line with an "and". I assume python does not evaluate the second expression if the first expression is false. Don't need to fix it now but seems more concise

LGraber · 2016-10-07T18:41:09Z

tableauserverclient/server/request_factory.py

+            if interval_item.end_time:
+                frequency_element.attrib['end'] = str(interval_item.end_time)
+        if hasattr(interval_item, 'interval'):
+            if interval_item.interval:


LGraber · 2016-10-07T18:41:26Z

tableauserverclient/server/request_factory.py

-        if interval_item.end_time:
-            frequency_element.attrib['end'] = str(interval_item.end_time)
+        if hasattr(interval_item, 'end_time'):
+            if interval_item.end_time:


LGraber · 2016-10-07T18:43:18Z

I think there is probably some more cleanup we can do but I am happy with where we have ended up. 🚀

t8y8 · 2016-10-07T18:44:50Z

🎉

@lbrendanl

* Adding contributing.md first draft * Adding information about Issues, Feature requests, and PRs Based on feedback from @lbrendanl * Adding additional detail to the the feature PR section based on feedback from @benlower * fixing typo

shinchris and others added 4 commits September 13, 2016 15:27

added ability to query and delete schedules

03e8ce3

added ability to create and update schedules

312a1c7

Merge branch 'development' of https://github.com/tableau/server-clien…

b32a632

…t-python into 43-schedules-support

Resubmitting tableau#30 with a few changes.

0adb03f

t8y8 commented Oct 4, 2016

View reviewed changes

graysonarts approved these changes Oct 4, 2016

View reviewed changes

Use new validation decorators

66e1793

t8y8 commented Oct 4, 2016

View reviewed changes

graysonarts approved these changes Oct 4, 2016

View reviewed changes

graysonarts mentioned this pull request Oct 4, 2016

added support for schedules #30

Closed

t8y8 added 2 commits October 4, 2016 18:53

Refactoring intervals into their own classes - Round 1

0342bed

And fix pep8

bb9ff8f

Refactor intervals - Round 2. Added validation back

34457b3

t8y8 commented Oct 5, 2016

View reviewed changes

Round 3, refactoring setters on the IntervalTypes. Import cleanup

f4490fb

t8y8 commented Oct 5, 2016

View reviewed changes

t8y8 added 2 commits October 5, 2016 14:43

Fix sample

79271a1

Round 4. Fully remove IntervalItem base class. Now it's just an enum …

b710d9d

…for various interval constants

t8y8 commented Oct 6, 2016

View reviewed changes

Minor cleanup

46cc1d0

LGraber reviewed Oct 6, 2016

View reviewed changes

Respond to CR feedback.

fd17154

t8y8 commented Oct 6, 2016

View reviewed changes

LGraber reviewed Oct 7, 2016

View reviewed changes

t8y8 added 2 commits October 6, 2016 22:04

Final round of cleanup, I think

371b6f9

Remove frequency and interval_occurence from public API

28ebb9e

LGraber reviewed Oct 7, 2016

View reviewed changes

t8y8 added 2 commits October 7, 2016 00:04

More cleanup

c330687

Move the interval-occurence tuple logic into a function just for the …

d7add3a

…serializer and keep the interval round-trippable now for all interval types. Minor cleanup. Note there's still casting between types because of how we parse from the XML, I will clean that up in another PR

LGraber reviewed Oct 7, 2016

View reviewed changes

t8y8 merged commit 5dbdbee into tableau:development Oct 7, 2016

t8y8 deleted the 43-schedules-support branch October 7, 2016 18:47

t8y8 mentioned this pull request Oct 7, 2016

Add support for Schedules #43

Closed

		@@ -0,0 +1,236 @@
		import xml.etree.ElementTree as ET
		from .interval_item import *

Adding Schedules Support Round 2 #48

Adding Schedules Support Round 2 #48

Conversation

t8y8 commented Oct 3, 2016 • edited

Choose a reason for hiding this comment

graysonarts left a comment

Choose a reason for hiding this comment

graysonarts Oct 4, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graysonarts left a comment

Choose a reason for hiding this comment

LGraber commented Oct 4, 2016

t8y8 commented Oct 4, 2016 • edited

graysonarts commented Oct 4, 2016

t8y8 commented Oct 5, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LGraber commented Oct 5, 2016

t8y8 commented Oct 5, 2016

LGraber commented Oct 5, 2016

t8y8 commented Oct 5, 2016

LGraber commented Oct 5, 2016

t8y8 commented Oct 5, 2016

LGraber commented Oct 5, 2016

t8y8 commented Oct 5, 2016

Choose a reason for hiding this comment

t8y8 Oct 6, 2016 • edited

Choose a reason for hiding this comment

graysonarts commented Oct 6, 2016

@t8y8 commented on this pull request.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8y8 Oct 6, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8y8 Oct 6, 2016 • edited

Choose a reason for hiding this comment

t8y8 Oct 6, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8y8 Oct 7, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8y8 commented Oct 3, 2016 •

edited

graysonarts Oct 4, 2016 •

edited

t8y8 commented Oct 4, 2016 •

edited

t8y8 commented Oct 5, 2016 •

edited

t8y8 Oct 6, 2016 •

edited

t8y8 Oct 6, 2016 •

edited

t8y8 Oct 6, 2016 •

edited

t8y8 Oct 6, 2016 •

edited

t8y8 Oct 7, 2016 •

edited

t8y8 Oct 7, 2016 •

edited