fix: Added Redshift and Spark typecheck to data_source event_timestamp_col inference

[shaibruhis]

What this PR does / why we need it:
RedshiftSource is a supported data_source, but the update_data_sources_with_inferred_event_timestamp_col() method omits it as a valid value in the assertion. This was causing the assertion to fail when using a RedshiftSource
Which issue(s) this PR fixes:

Fixes #

[adchia]

/ok-to-test

[adchia]

Thanks for catching and making a PR!

Mind updating to master? Also mind adding a test e.g. at https://github.com/feast-dev/feast/blob/master/sdk/python/tests/integration/registration/test_inference.py#L87-L87 ?

[codecov-commenter]

Codecov Report

Merging #2389 (e72c216) into master (c2389da) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2389      +/-   ##
==========================================
+ Coverage   85.47%   85.49%   +0.01%     
==========================================
  Files         122      122              
  Lines       10714    10727      +13     
==========================================
+ Hits         9158     9171      +13     
  Misses       1556     1556              

Flag	Coverage Δ
integrationtests	75.34% <100.00%> (-0.19%)	⬇️
unittests	57.08% <42.85%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdk/python/feast/inference.py	83.82% <ø> (ø)
...ts/integration/feature_repos/repo_configuration.py	96.10% <100.00%> (+0.05%)	⬆️
...n/tests/integration/registration/test_inference.py	100.00% <100.00%> (ø)
sdk/python/feast/go_server.py	89.18% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2389da...e72c216. Read the comment docs.

[shaibruhis]

Hey Danny! Not sure what you mean by "updating to master", I set the base to master?

Also, the test you listed already checks the RedshiftSource. I think the right test here is to add a RedshiftSource in here: https://github.com/feast-dev/feast/blob/master/sdk/python/tests/integration/registration/test_inference.py#L119-L123. I saw there is a utils class RedshiftDataSourceCreator but curious if there is a test Redshift data source?

[adchia]

Hey Danny! Not sure what you mean by "updating to master", I set the base to master?

Also, the test you listed already checks the RedshiftSource. I think the right test here is to add a RedshiftSource in here: https://github.com/feast-dev/feast/blob/master/sdk/python/tests/integration/registration/test_inference.py#L119-L123. I saw there is a utils class RedshiftDataSourceCreator but curious if there is a test Redshift data source?

Ah sorry I was unclear. I meant pulling the latest changes from master.

With the test, I accidentally referenced a specific line there, but was referring to the overall file. There is a test Redshift data source available actually. You'll want to probably pass in the test fixtures we generate (e.g. https://github.com/feast-dev/feast/blob/master/sdk/python/tests/integration/feature_repos/repo_configuration.py#L222-L222 are test sources we generate) -> an example of where we pull this is in test_historical_features where it passes in the universal_data_sources test fixture. That gives you access to the data sources defined.

Let me know if you have any questions!

[shaibruhis]

Hey @adchia, anything else i should do to make land?

[adchia]

/lgtm

[adchia]

Looks like you need to pass the linter by sorting.

You can run this linter locally with make lint-python. You also should probably install the precommit hooks (https://github.com/feast-dev/feast/blob/master/CONTRIBUTING.md#pre-commit-hooks) so this is run automatically

[adchia]

The linter looks now to be failing and your commit wasn't signed?

If you give me push privileges to your branch btw happy to also help resolve quickly.

[shaibruhis]

Fixed the lint issues! I realized I had the wrong linter set which was causing problems for me. Apologies for that

[adchia]

/lgtm

[adchia]

/lgtm

[feast-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, shaibruhis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [adchia]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment