Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Timezone-Sensitive GH-Actions #828

Merged
merged 11 commits into from
Oct 19, 2022
Merged

[HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Timezone-Sensitive GH-Actions #828

merged 11 commits into from
Oct 19, 2022

Conversation

tdoehmen
Copy link
Contributor

@tdoehmen tdoehmen commented Oct 13, 2022 •
edited
Loading

This PR fixes..

  • timezone-independent conversion of hudi timestamps
  • adds timezone-senstive tests for ubuntu and windows to CI

JIRA Issue: -

Priority for Review: -

Related PRs: -

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Tests on VM

Checklist For The Assigned Reviewer:

- [ ] Checked if merge conflicts with master exist
- [ ] Checked if stylechecks for Java and Python pass
- [ ] Checked if all docstrings were added and/or updated appropriately
- [ ] Ran spellcheck on docstring
- [ ] Checked if guides & concepts need to be updated
- [ ] Checked if naming conventions for parameters and variables were followed
- [ ] Checked if private methods are properly declared and used
- [ ] Checked if hard-to-understand areas of code are commented
- [ ] Checked if tests are effective
- [ ] Built and deployed changes on dev VM and tested manually
- [x] (Checked if all type annotations were added and/or updated appropriately)

@tdoehmen tdoehmen requested a review from kennethmhc October 13, 2022 14:38
@tdoehmen tdoehmen changed the title [HOPSWORKS-3326][Append] Timezone-unaware conversion of Hudi Timestamps [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps Oct 13, 2022
@tdoehmen tdoehmen requested a review from davitbzh October 17, 2022 15:36
kennethmhc
kennethmhc approved these changes Oct 18, 2022
View reviewed changes
Copy link
Contributor

@kennethmhc kennethmhc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kennethmhc
Copy link
Contributor

It seems that a unit test is missing.

@tdoehmen
Copy link
Contributor Author

tdoehmen commented Oct 18, 2022 •
edited
Loading

That's a good point! I'd need to change the python session's timezone for a good test, but the only way I can find to do this, only works on unix, so this would kind of break the tests on windows

os.environ['TZ'] = 'Europe/London'
time.tzset()`

Alternative could be to make an additional CI run with a GH Actions in a different timezone. Then we also woulndn't need addional tests, because the function itself is already covered by some tests.

@kennethmhc
Copy link
Contributor

I'd need to change the python session's timezone for a good test, but the only way I can find to do this, only works on unix, so this would kind of break the tests on windows

Sounds good, since the CICD pipeline is unix. But can you skip the test on Windows?

@tdoehmen
Copy link
Contributor Author

I added actions for local timezones on windows as well as ubuntu

@tdoehmen tdoehmen changed the title [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Added Timezone-Sensitive Tests to CI Oct 19, 2022
@tdoehmen tdoehmen changed the title [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Added Timezone-Sensitive Tests to CI [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Timezone-Sensitive GH-Actions Oct 19, 2022
@tdoehmen tdoehmen merged commit 477f5b7 into logicalclocks:master Oct 19, 2022
davitbzh added a commit to davitbzh/feature-store-api that referenced this pull request Oct 20, 2022
@tdoehmen @moritzmeister
@SirOibaf @dhananjay-mk @kennethmhc @davitbzh
Datetime format tests (#10)
5efc7ef 
* [FSTORE-11] Add mkdocs macros dependency for docs (logicalclocks#800)

* [HOPSWORKS-3296] Remove update of storage connector (logicalclocks#801)

* [FSTORE-329] CommitDTO type field is not returned anymore after Payar… (logicalclocks#805)

* [FSTORE-8] add query default argument in snowflake (logicalclocks#790)

* [FSTORE-8][append] Add unit test (logicalclocks#808)

* [FSTORE-33] Improve HSFS parameter description (logicalclocks#806)

* [FSTORE-331] BigQuery connector doesn't work if the application is ex… (logicalclocks#809)

* [FSTORE-331] BigQuery connector doesn't work if the application is executed on multiple nodes

* Change the BigQuery configuration so that it uses the credential field
  (base64 encoding of the content of the credential file)
  instead of providing the path in the credentialFile property.

* For Java, updated the tests to use JUnit5 to allow for the @tmpdir
  annotation

* Check if options is null before checking if it's empty

* [FSTORE-8][append] Java fix for snowflake connector (logicalclocks#810)

* [FSTORE-345] Update documentation to reflect supported methods in hsfs engines (logicalclocks#814)

* [HOPSWORKS-3345] Improve event time error message (logicalclocks#785)

* improve error message

* [FSTORE-348] Refactor Tutorials documentation page (logicalclocks#817)

* fix style (logicalclocks#820)

* [FSTORE-358] Add icon to external links in documentation navigation (logicalclocks#822)

* [FSTORE-311] Improved timestamp with timezone handling (logicalclocks#821)

* add timezone normalization to python engine

* set spark session timezone to utc

* stylecheck

* set spark timezone in java client

* style

* remove spark proerty validation because its internally set as session property

* [FSTORE-342] Add java tests to github pipeline (logicalclocks#819)

* [FSTORE-342] Add java tests to github pipeline

* Fix Java build

* Add m2 caching

* Remove failure ignore flag

* [FSTORE-346] Handle as_of in feature view (logicalclocks#816)

* handle as_of in feature view

* fix style

* add unit test

* fix style

* fix tests

* fix style

* fix style

* refactor is_time_travel

* address comments

* fix style

* [FSTORE-331][Append] Fix Storage Connector Tests on windows (logicalclocks#827)

* [FSTORE-383] Feature View method documentation (logicalclocks#831)

* [FSTORE-311][Append] Timezone-aware Timestamp conversion for PySpark client (logicalclocks#826)

* fixed timezones for pyspark clients in different timezone

* spark session timezone to utc

* [FSTORE-359] Rename Transformation Function Output Types and Fix Timezone-related issues (logicalclocks#829)

* remove unused import

* spark types for tf

* spark types for tf

* tmp

* merge rebase with upstream/master

* [HOPSWORKS-3342][Append] unit tests (#9)

* added unit tests for transformation functions

* fixed stylecheck

* fixed a small thing in docs

* infer_python_type

* fix tests

* fix tests

* fix checks

* removed legacy types cast from test

* add convert_column method, fix style

* updated builtin transformation functions

* style

(cherry picked from commit 6ad26c4cb72cfd286dbb3c7049cd72f0aed3ab78)

* made transformation functions timezone-safe

(cherry picked from commit 176c2da134517648d568f69784b1e13c56c580f1)

* style

* removed deprecated np types

* tests tf function returning none

* style

* fixed tests

* style

Co-authored-by: davitbzh <davit.bzhalava@gmail.com>

* [HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Timestamps and Timezone-Sensitive GH-Actions (logicalclocks#828)

* made conversion from utc explicit

* windows and timezone action

* rename action

* change runner to windows

* removed python variants from tz-local actions

* added java tz local action

* small rename

* added tests

* remove unused variable

* set correct timezone in java tests

* [FSTORE-332] implement method for get_or_create_feature_view (logicalclocks#813)

* get_or_create_feature_view

* uppercased Y

* updated comments

* format

* added some basic tests

Co-authored-by: Moritz Meister <8422705+moritzmeister@users.noreply.github.com>
Co-authored-by: Fabio Buso <buso.fabio@gmail.com>
Co-authored-by: Dhananjay Mukhedkar <55157590+DhananjayMukhedkar@users.noreply.github.com>
Co-authored-by: kennethmhc <kennethmhc@users.noreply.github.com>
Co-authored-by: davitbzh <davit.bzhalava@gmail.com>
Co-authored-by: davitbzh <44586065+davitbzh@users.noreply.github.com>
kennethmhc pushed a commit to kennethmhc/feature-store-api that referenced this pull request Nov 16, 2022
@tdoehmen @kennethmhc
[HOPSWORKS-3326][Append] Timezone-independent conversion of Hudi Time…
ea37a3b 
…stamps and Timezone-Sensitive GH-Actions (logicalclocks#828)

* made conversion from utc explicit

* windows and timezone action

* rename action

* change runner to windows

* removed python variants from tz-local actions

* added java tz local action

* small rename

* added tests

* remove unused variable

* set correct timezone in java tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennethmhc kennethmhc kennethmhc approved these changes

@davitbzh davitbzh Awaiting requested review from davitbzh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tdoehmen @kennethmhc