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

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

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

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?

core/src/klio_core/config/_io.py Show resolved Hide resolved
core/src/klio_core/config/_io.py Outdated Show resolved Hide resolved
@shireen-bean shireen-bean changed the title [core] Add KlioWriteToText missing fields including file_name_suffix,… [WIP][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,… [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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants