✨ Source Zendesk Support: add tests and clean-up #55676
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/source.py
Show resolved
Hide resolved
...ctors/source-zendesk-support/unit_tests/integrations/zs_responses/records/records_builder.py
Show resolved
Hide resolved
| response["meta"]["has_more"] = True | ||
| response["meta"]["after_cursor"] = "after-cursor" | ||
| response["meta"]["before_cursor"] = "before-cursor" | ||
| if self._first_url: |
There was a problem hiding this comment.
The low-code version implemented a links_next_paginator that relies on links.next and not on meta.after_cursor hence I updated this strategy to support both
octavia-squidington-iii
added
the
area/documentation
| @@ -32,6 +32,8 @@ acceptance_tests: | |||
| bypass_reason: "not available in current subscription plan" | |||
| - name: "post_comment_votes" | |||
| bypass_reason: "not available in current subscription plan" | |||
| - name: "ticket_activities" | |||
There was a problem hiding this comment.
Based on https://connectors.airbyte.com/files/generated_reports/test_summary/source-zendesk-support/index.html, this is failing for a week now. The retention period is 30 days which means we would need to create some data every 30 days for this stream. I prefer just disabling it for now
| from .zs_responses.records import PostsRecordBuilder | ||
|
|
||
|
|
||
| _NOW = datetime.now(timezone.utc) |
There was a problem hiding this comment.
maybe a dumb question, but why do we need to use two different libraries to get _NOW and _START_DATE?
are we be unable to use pendulum.now(tz="UTC") when we convert to isoformat() in the freezegun decorator?
There was a problem hiding this comment.
I don't know but I think that our reasoning should be to move away from pendulum because it's not supported anymore. I haven't done so because there are a lot of utils relying on it. I wanted to use Devin and try to ask it to move remove all the usages of pendulum eventually. I kicked off the job but didn't check the result yet...
| @@ -261,7 +261,7 @@ def get_stream_state_value(self, stream_state: Mapping[str, Any] = None) -> int: | |||
| Returns the state value, if exists. Otherwise, returns user defined `Start Date`. | |||
| """ | |||
| state = stream_state.get(self.cursor_field) or self._start_date if stream_state else self._start_date | |||
| return calendar.timegm(pendulum.parse(state).utctimetuple()) | |||
| return int(state) if isinstance(state, int) or state.isdigit() else calendar.timegm(pendulum.parse(state).utctimetuple()) | |||
There was a problem hiding this comment.
I don't have an issue w/ the code, but i'm interested in how this got surfaced. Was there a customer that saw this as an issue or was this just found when you were writing mock server tests?
There was a problem hiding this comment.
It is not a bug. It is to have this test pass. However, there are so few users in prod that I don't think we intend to revert. We could just remove that honestly...
What
Partially addresses https://github.com/airbytehq/airbyte-internal-issues/issues/11894
Preparing for the migration to low-code:
How
For the posts stream supporting low-code state, see
streams.pyandtest_given_input_state_with_low_code_format_when_read_then_set_state_value_to_most_recent_cursor_valueReview guide
User Impact
Can this PR be safely reverted and rolled back?