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

Test_data added for #4764 #1764

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

10sharmashivam
Copy link
Contributor

Tracking issue

Reference Issue
Reference PR - In which adding .csv files allowed

Why are the changes needed?

These changes help and keep test_data on flyteorg/flytesnacks repo, which can work as internal source for test data

Once this PR is approved and merged, I will use the raw github user content link of these test data, and use to correct the broken tests.

Signed-off-by: 10sharmashivam <[email protected]>
Signed-off-by: 10sharmashivam <[email protected]>
@davidmirror-ops
Copy link
Contributor

@10sharmashivam this is great! does the example work for you in your local cluster?

@10sharmashivam
Copy link
Contributor Author

@10sharmashivam this is great! does the example work for you in your local cluster?

Hi @davidmirror-ops! Yes, the example worked correctly for me. I temporarily hosted the biostats.csv file using a local HTTP server, which allowed me to access the CSV file via a localhost URL. I used this URL as the csv_url input to test the CSV normalization, addressing the issue with the broken external link in the parent issue. It produced the expected output—a normalized CSV file—so everything worked perfectly.

@10sharmashivam
Copy link
Contributor Author

Hi @davidmirror-ops, I wanted to check in to see if there’s anything else I can provide for this PR. Please let me know if there are any other tests or adjustments needed. Thanks!

Copy link
Contributor

@davidmirror-ops davidmirror-ops left a comment

Choose a reason for hiding this comment

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

Thanks

@davidmirror-ops davidmirror-ops merged commit 2ab1024 into flyteorg:master Oct 30, 2024
60 checks passed
@10sharmashivam 10sharmashivam mentioned this pull request Oct 30, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants