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

DATA-2006 - Remove filter from SubmitTrainingJobRequest in API #402

Merged
merged 7 commits into from
Dec 4, 2023

Conversation

tahiyasalam
Copy link
Member

@tahiyasalam tahiyasalam commented Nov 27, 2023

I cross-referenced this PR against both https://github.com/viamrobotics/app/pull/3010 and viamrobotics/rdk#3267, where tests in all scenarios and there was no reference to the GetFilter field. Once merged, I will double pull into app and complete the last change which is removing the filter: undefined field from the FE

This reverts commit 21a96c4.
@tahiyasalam tahiyasalam requested a review from agreenb November 29, 2023 21:17
Copy link
Contributor

@agreenb agreenb left a comment

Choose a reason for hiding this comment

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

LGTM, can get approval from Simone or Steve and make sure to merge both this and the app update before tomorrow's deployment.

@tahiyasalam
Copy link
Member Author

@Simberific would you mind manually merging? This is based on this scope doc, where we decided we would disallow training on data by filter and only support training on data in dataset.

@Simberific
Copy link
Member

Simberific commented Dec 4, 2023 via email

@Simberific
Copy link
Member

Looks like I'm currently member - just asked Steve to bump my permissions so I can squash & merge.

@Simberific Simberific merged commit c3fd49f into viamrobotics:main Dec 4, 2023
1 check failed
@agreenb
Copy link
Contributor

agreenb commented Dec 4, 2023 via email

@Simberific
Copy link
Member

Done!

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.

4 participants