Skip to content

[core] Add KlioWriteToText missing fields including file_name_suffix,…#200

Open
shireen-bean wants to merge 3 commits intodevelopfrom
fix_write_to_text_suffix
Open

[core] Add KlioWriteToText missing fields including file_name_suffix,…#200
shireen-bean wants to merge 3 commits intodevelopfrom
fix_write_to_text_suffix

Conversation

@shireen-bean
Copy link
Contributor

Changes

Add KlioWriteToText missing fields including file_name_suffix, num_shards, append_trailing_newlines etc

A user reported his job erroring out when providing file_name_suffix when default event output was enabled in batch mode.
This PR enables missing fields for KlioWriteToText that are available in the beam conterpart.

Testing

Added a test use case for having more than the bare minimum configuration fields for the KlioWriteFileConfig.
Tested this on a job using klio devtools for a test batch job. Tested to ensure that the job would continue to work with just:

  events:
    inputs:
       <- snip ->
    outputs:
      - type: file
        location: gs://shireen-batch-test/

as well as work with the config:

  events:
    inputs:
       <- snip ->
    outputs:
      - type: file
        location: gs://shireen-batch-test/
        file_name_suffix: .txt
        num_shards: 3
        append_trailing_newlines: False

Checklist for PR author(s)

  • Format the pull request title like [cli] Fixes bugs in 'klio job fake-cmd'.
  • Changes are covered by unit tests (no major decrease in code coverage %) and/or integration tests.
  • Document any relevant additions/changes in the appropriate spot in docs/src.
  • For any change that affects users, update the package's changelog in docs/src/reference/<package>/changelog.rst.
Copy link
Contributor

@econchick econchick left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! Looks like a workflow/test is failing and needs fixing. Just had a couple of questions, otherwise it looks good.

Not sure when to release it though - maybe in 21.7.0.alpha1?

@shireen-bean shireen-bean changed the title [core] Add KlioWriteToText missing fields including file_name_suffix,… Jul 2, 2021
@shireen-bean shireen-bean force-pushed the fix_write_to_text_suffix branch from 84b0f1d to 1d11f3d Compare July 6, 2021 13:30
@shireen-bean shireen-bean changed the title [WIP][core] Add KlioWriteToText missing fields including file_name_suffix,… Jul 12, 2021
@shireen-bean shireen-bean force-pushed the fix_write_to_text_suffix branch 2 times, most recently from f357e90 to e34273a Compare August 4, 2021 22:13
@shireen-bean shireen-bean force-pushed the fix_write_to_text_suffix branch from e34273a to 0fc3219 Compare August 4, 2021 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants