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

#2175: Add ability to configure how spark handles dates in parquet files #2184

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

#2175: Add ability to configure how spark handles dates in parquet files #2184

wants to merge 4 commits into from

Conversation

TebaleloS
Copy link
Collaborator

@TebaleloS TebaleloS commented Mar 23, 2023 •
edited by miroslavpojer
Loading

Notes

  • Initialized DEFAULT_PARQUET_DATETIME_READ_MODE and DEFAULT_PARQUET_DATETIME_WRITE_MODE with a default value CORRECTED.
  • Added option condition for assessing the value of datetime read/write mode

Closes #2175

@TebaleloS TebaleloS marked this pull request as ready for review March 23, 2023 07:41
miroslavpojer
miroslavpojer reviewed Mar 23, 2023
View reviewed changes
Copy link
Collaborator

@miroslavpojer miroslavpojer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • pulled
  • code review
  • command generation test

Used command-line calls:

  • run_conformance.sh --rest-api-credentials-file ~/.ssh/menasCredential.properties --dataset-name test --dataset-version 1 --report-date 2020-03-03 --dry-run
  • run_standardization.sh --rest-api-credentials-file ~/.ssh/menasCredential.properties --dataset-name test --dataset-version 1 --report-date 2020-03-03 --dry-run
  • run_standardization_conformance.sh --rest-api-credentials-file ~/.ssh/menasCredential.properties --dataset-name test --dataset-version 1 --report-date 2020-03-03 --dry-run

Only bash files file were tested.
There is required to test last cmd one.

dk1844
dk1844 approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

LGTM

  • code reviewed
  • pulled
  • prepared dummy Enceladus env to see the spark-submit command
  • run

@dk1844
Copy link
Contributor

dk1844 commented Mar 24, 2023 •
edited
Loading

I have tested both the sh and cmd script to see that they correctly pass the datetime configuration fields down to the spark-submit. I have not run the actual job.

sh

./run_standardization.sh ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY -> spark-submit ... --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ... ✔️

./run_conformance.sh ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY -> spark-submit ... --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ... ✔️

./run_standardization_conformance.sh ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY -> spark-submit ... --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ... ✔️

cmd

.\run_standardization.cmd ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY ->
spark-submit ... --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ... ✔️

.\run_conformance.cmd ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY ->
spark-submit --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ✔️

.\run_standardization_conformance.cmd ... --parquet-datetime-read-mode XX --parquet-datetime-write-mode YY -> spark-submit" ... --conf spark.sql.parquet.datetimeRebaseModeInRead=XX --conf spark.sql.parquet.datetimeRebaseModeInWrite=YY --conf spark.sql.parquet.int96RebaseModeInRead=XX --conf spark.sql.parquet.int96RebaseModeInWrite=YY ... ✔️

@dk1844 dk1844 added the PR:tested Only for PR - PR was tested by a tester (person) label Mar 24, 2023
@sonarcloud
Copy link

sonarcloud bot commented Mar 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@benedeki benedeki mentioned this pull request Apr 16, 2023
Open
lsulak
lsulak approved these changes May 3, 2023
View reviewed changes
Copy link
Collaborator

@lsulak lsulak left a comment

Choose a reason for hiding this comment

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

LGTM, just read the code, given that two reviewers tested it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miroslavpojer miroslavpojer miroslavpojer approved these changes

@lsulak lsulak lsulak approved these changes

@dk1844 dk1844 dk1844 approved these changes

@benedeki benedeki Awaiting requested review from benedeki benedeki is a code owner

@jakipatryk jakipatryk Awaiting requested review from jakipatryk jakipatryk is a code owner

Assignees
No one assigned
Labels
PR:tested Only for PR - PR was tested by a tester (person)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to configure how Spark handles dates in parquet files.
4 participants
@TebaleloS @dk1844 @lsulak @miroslavpojer