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

MTC Improve Error Messages for Normalize Transform [DT-409] #598

Merged
merged 8 commits into from
Jun 21, 2024
Merged

MTC Improve Error Messages for Normalize Transform [DT-409] #598

merged 8 commits into from
Jun 21, 2024

Conversation

binh-dam-ibigroup
Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup commented Jun 6, 2024 •
edited
Loading

Checklist

  • Appropriate branch selected => This is a PR against the MTC branch.
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • [na] The description lists all applicable issues this PR seeks to resolve
  • [na] The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

This PR improves error messages for common cases when the Normalize Field Transform fails:

  • A corrupt ZIP file is submitted for transformation.
  • A non-GTFS but valid ZIP file is submitted for transformation.
  • A GTFS ZIP file is submitted for transformation but the expected table file or field is missing.
  • The field name parameter for the transformation is not set (i.e. a case where someone started adding a Normalize Field transformation but did not finish configuring it and proceeded to import a GTFS feed).

@binh-dam-ibigroup binh-dam-ibigroup changed the title MTC Improve Error Messages for Normailize Transform [DT-409] MTC Improve Error Messages for Normalize Transform [DT-409] Jun 6, 2024
@binh-dam-ibigroup binh-dam-ibigroup requested a review from br648 June 6, 2024 20:14
br648
br648 approved these changes Jun 7, 2024
View reviewed changes
Copy link
Contributor

@br648 br648 left a comment

Choose a reason for hiding this comment

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

One very minor comment. I can't run the unit test because of OS and file locking. Useful update though, nice one!

src/main/java/com/conveyal/datatools/manager/models/transform/NormalizeFieldTransformation.java Outdated Show resolved Hide resolved
@br648 br648 assigned binh-dam-ibigroup and unassigned br648 Jun 7, 2024
josh-willis-arcadis
josh-willis-arcadis approved these changes Jun 12, 2024
View reviewed changes
Copy link

@josh-willis-arcadis josh-willis-arcadis left a comment

Choose a reason for hiding this comment

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

LGTM

@binh-dam-ibigroup binh-dam-ibigroup merged commit 4528783 into mtc-deploy Jun 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josh-willis-arcadis josh-willis-arcadis josh-willis-arcadis approved these changes

@br648 br648 br648 approved these changes

Assignees

@binh-dam-ibigroup binh-dam-ibigroup

Labels
Pending ext. approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@binh-dam-ibigroup @br648 @josh-willis-arcadis