edanalytics / earthmover

CLI tool for transforming collections of tabular source data into a variety of text-based data formats via YAML configuration and Jinja templates.

Apache License 2.0

19 stars 2 forks source link

Feature: Refactor Destination Execute #95

Closed jayckaiser closed 3 months ago

jayckaiser commented 3 months ago

Feature: Refactor Destination Execute

Description & motivation

This branch makes the following changes to FileDestination:

Refactor all field-parsing to use ErrorHandler.assert_get_key().
Move template-file checking into execute().
Unify template processing into a single try-except with multiple Exception clauses.

When running earthmover compile, we want the compile process to only parse the Earthmover template, not check for the existence of files.

PR Merge Priority:

[ ] Low
[ ] Medium
[x] High

This aligns with some execute cleanup goals with other hotfix branches for RC/0.3.2.

Tests and QC done:

All built-in tests have been successfully rerun with these changes.

tomreitz commented 3 months ago

@jayckaiser this looks fine to me, but

the PR is against main, should the base be updated to rc/0.3.2?
it will run into a merge conflict with my change (already merged to rc/0.3.2) to how destination output files are written

If you update based on the above, I'll re-review and approve. Thanks!