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

Fix avro files read failure under timePartitionPattern paths #808

Merged
merged 17 commits into from
Nov 14, 2022

Conversation

enya-yx
Copy link
Collaborator

@enya-yx enya-yx commented Nov 1, 2022

Description

  • Modify test cases to test 'timePartitionPattern' e2e

How was this PR tested?

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to clarify your proposed changes.

@enya-yx enya-yx added the safe to test Tag to execute build pipeline for a PR from forked repo label Nov 1, 2022
@enya-yx enya-yx changed the title Fix avro files read failure under timePartitionPattern paths [ WIP ] Fix avro files read failure under timePartitionPattern paths Nov 2, 2022
Yuqing-cat
Yuqing-cat previously approved these changes Nov 3, 2022
@blrchen
Copy link
Collaborator

blrchen commented Nov 3, 2022

Can docs get updated to give samples on how to use this new params?

@enya-yx
Copy link
Collaborator Author

enya-yx commented Nov 7, 2022

Can docs get updated to give samples on how to use this new params?

The comments under definition of 'HdfsSource' is the related document update and it can be shown through API document link we exposed.

@blrchen
Copy link
Collaborator

blrchen commented Nov 7, 2022

Can docs get updated to give samples on how to use this new params?

The comments under definition of 'HdfsSource' is the related document update and it can be shown through API document link we exposed.

Yes, that is what i meant, we should put that in docs because user normally do not read our test code, they read docs.

@enya-yx
Copy link
Collaborator Author

enya-yx commented Nov 8, 2022

Can docs get updated to give samples on how to use this new params?

The comments under definition of 'HdfsSource' is the related document update and it can be shown through API document link we exposed.

Yes, that is what i meant, we should put that in docs because user normally do not read our test code, they read docs.

The comment was already added in the file 'feathr_project/feathr/definition/source.py'

@blrchen
Copy link
Collaborator

blrchen commented Nov 8, 2022

Can docs get updated to give samples on how to use this new params?

The comments under definition of 'HdfsSource' is the related document update and it can be shown through API document link we exposed.

Yes, that is what i meant, we should put that in docs because user normally do not read our test code, they read docs.

The comment was already added in the file 'feathr_project/feathr/definition/source.py'

I see why you are confused here, might be I didn't explain clearly enough, I meant public docs like this one https://feathr-ai.github.io/feathr/how-to-guides/feathr-input-format.html should be updated for use directories as input source.

@enya-yx enya-yx merged commit 14f0f12 into feathr-ai:main Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Tag to execute build pipeline for a PR from forked repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants