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.

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

CI/CD overhaul #403

Merged
merged 141 commits into from
Jul 11, 2024
Merged

CI/CD overhaul #403

merged 141 commits into from
Jul 11, 2024

Conversation

jtisbell4
Copy link
Contributor

@jtisbell4 jtisbell4 commented Apr 28, 2024

Major changes to test and build infrastructure, predominantly related to tox:

  • tox environments correctly configured
  • GH actions simplified/refactored to exclusively use tox
  • Officially dropped support for DBR < 11.3
  • Moved DBR-specific requirements into python/tests/requirements
  • Made SparkSession initialization dynamic in test class setup

A few minor changes in python/tempo:

  • Fixed if statement in TSDF.withRangeStats
  • Removed event_time column assumption and made Z-ORDER attempt contingent on optimizationCols in io.write

Copy link
Contributor

@tnixon tnixon left a comment

Choose a reason for hiding this comment

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

Minor change needed in build-release.yml workflow, but o/w LGTM! 😄 🕺🏻 🎉

.github/workflows/push.yml Outdated Show resolved Hide resolved
.github/workflows/build-release.yml Show resolved Hide resolved
@R7L208 R7L208 requested a review from tnixon July 11, 2024 13:29
@tnixon tnixon requested a review from R7L208 July 11, 2024 18:36
url: https://help.databricks.com/
about: Issues related to Databricks and not related to UCX

- name: UCX Documentation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: UCX Documentation
- name: Tempo Documentation

contact_links:
- name: General Databricks questions
url: https://help.databricks.com/
about: Issues related to Databricks and not related to UCX
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
about: Issues related to Databricks and not related to UCX
about: Issues related to Databricks and not related to Tempo

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this got accidentally checked in while I was working. @jtisbell4 - did you add this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you allow minor releases on the requirement files so we are able to automatically pick up patches that have bug and security fixes?

def test_tsdf_interpolate(self):
...
def test_tsdf_interpolate(self): ...
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this test since it's not implemented?

"my_table_optimization_col_fails zorder by (symbol,date,event_time)\n^^^\n"
],
)
if pkg_version.parse(DELTA_VERSION) < pkg_version.parse("2.0.0"):
Copy link
Contributor

Choose a reason for hiding this comment

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

@jtisbell4 - can we mock this env var so there's no conditional logic in the test? the if/else will make it conditionally fail/pass but the test outcome should be consistent regardless of delta version installed

@R7L208 R7L208 merged commit 2c97b42 into master Jul 11, 2024
9 checks passed
@R7L208 R7L208 deleted the tox-refactor branch July 11, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants