Fix ValueType.UNIX_TIMESTAMP conversions

[judahrand]

Signed-off-by: Judah Rand [email protected]

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #2209

Does this PR introduce a user-facing change?:

Fix ValueType.UNIX_TIMESTAMP conversions

[adchia]

/lgtm

[adchia]

Is there a way to capture this in a test somehow?

[codecov-commenter]

Codecov Report

Merging #2219 (f919811) into master (62fae05) will decrease coverage by 25.36%.
The diff coverage is 3.70%.

@@             Coverage Diff             @@
##           master    #2219       +/-   ##
===========================================
- Coverage   84.92%   59.56%   -25.37%     
===========================================
  Files         105      105               
  Lines        8498     8522       +24     
===========================================
- Hits         7217     5076     -2141     
- Misses       1281     3446     +2165     

Flag	Coverage Δ
integrationtests	?
unittests	59.56% <3.70%> (-0.16%)	⬇️

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

Impacted Files	Coverage Δ
sdk/python/feast/type_map.py	38.88% <0.00%> (-27.52%)	⬇️
sdk/python/tests/data/data_creator.py	34.78% <0.00%> (-65.22%)	⬇️
...s/integration/registration/test_universal_types.py	34.96% <10.00%> (-63.75%)	⬇️
.../integration/online_store/test_online_retrieval.py	17.39% <0.00%> (-82.61%)	⬇️
.../integration/online_store/test_universal_online.py	15.20% <0.00%> (-82.03%)	⬇️
...fline_store/test_universal_historical_retrieval.py	19.18% <0.00%> (-80.82%)	⬇️
sdk/python/tests/utils/online_read_write_test.py	20.68% <0.00%> (-79.32%)	⬇️
...gration/registration/test_feature_service_apply.py	31.25% <0.00%> (-68.75%)	⬇️
sdk/python/feast/infra/online_stores/redis.py	26.97% <0.00%> (-67.11%)	⬇️
sdk/python/tests/integration/e2e/test_usage_e2e.py	35.00% <0.00%> (-65.00%)	⬇️
... and 53 more

---

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 62fae05...f919811. Read the comment docs.

[judahrand]

Is there a way to capture this in a test somehow?

Hmmm... 🤔 I guess we just need to add a ValueType.UNIX_TIMESTAMP feature to the tests somewhere. I can do this - though in my experience adding new feature types to the tests is a chore 😛

[judahrand]

@adchia Added a datetime Feature to the tests. Hope this is sufficient.

[judahrand]

@adchia Looks likes there's some bugs in the tests that I don't think I'll be able to fix as I don't have access to the AWS or GCP test infrastructure.

[judahrand]

@adchia I can only imagine that this method is failing to create the table for Redshift:

feast/sdk/python/tests/integration/feature_repos/universal/data_sources/redshift.py

Line 34 in 62fae05

def create_data_source(

Are you able to help debug this as you have access to the Redshift side?

[adchia]

yeah let me take a look

[judahrand]

@adchia Did you get a chance to look at this at all? Ideally, I think it makes sense for this and #2229 to go in roughly at the same time as there will be merge conflicts so I'm kinda waiting to review that one until this one can move ahead.

[adchia]

I tried it and couldn't repro (i.e. the tests pass for me locally).

Trying it again now

[judahrand]

I tried it and couldn't repro (i.e. the tests pass for me locally).

Trying it again now

How odd... Maybe something to do with the credentials on the repo Vs local ones?

I think this is a really important fix as currently I'm not sure UNIX_TIMESTAMP features work end to end. Unfortunately, there's not much I can do on my end.

[adchia]

/lgtm

[feast-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

[felixwang9817]

cc @judahrand the root cause of the AWS failures on here should be fixed by #2242