-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Source SalesForce: move to next releaseStage #28243
Source SalesForce: move to next releaseStage #28243
Conversation
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
airbyte-integrations/connectors/source-salesforce/source_salesforce/streams.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-salesforce/source_salesforce/streams.py
Outdated
Show resolved
Hide resolved
(stream_slice or {}).get("start_date", ""), | ||
(next_page_token or {}).get("start_date", ""), | ||
) | ||
start_date = max((stream_state or {}).get(self.cursor_field, ""), (stream_slice or {}).get("start_date", "")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should handlet he case where both are ""
, the default value should be 2017-10-01
for this dev version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like this is an old implementation, we always have a stream slices here and therefore stream_slice.get("start_date")
will be enough
4bb15ea
to
bfdc7b6
Compare
airbyte-integrations/connectors/source-salesforce/source_salesforce/source.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
@@ -1,38 +1,17 @@ | |||
# Using alpine to remove several vulnerabilities frm slim image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted the changes in #28021 because it was causing the build time to be 4 hours long. We should revert this if needed until the contributor can find a way to make this faster.
…thub.com:airbytehq/airbyte into artem1205/source-salesforce-silver-certification
…er-certification' into artem1205/source-salesforce-silver-certification
….18" This reverts commit c7e14ac.
@@ -111,7 +111,8 @@ def generate_streams( | |||
start_date = config.get( | |||
"start_date", (datetime.now() - relativedelta(years=cls.START_DATE_OFFSET_IN_YEARS)).strftime(cls.DATETIME_FORMAT) | |||
) | |||
stream = incremental(**streams_kwargs, replication_key=replication_key, start_date=start_date) | |||
end_date = config.get("end_date") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be end date OR now()?
What
Resolve:
How
airbyte_cdk
versionRecommended reading order
x.java
y.python
🚨 User Impact 🚨
no breaking changes
Pre-merge Actions
Updating a connector
Community member or Airbyter
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.