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

Allow supplying custom workflow yaml for infer_with_model job #7902

Merged
merged 14 commits into from
Aug 5, 2024

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Jun 25, 2024

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

TODOs:

Issues:


  • Removed dev-only changes like prints and application.conf edits
  • Considered common edge cases

@philippotto
Copy link
Member

probably read for review now, right?

@fm3
Copy link
Member Author

fm3 commented Jul 22, 2024

probably read for review now, right?

The worker side isn’t ready, I’ll do some end-to-end testing next. I think that more checkboxes may arise (e.g. we will likely drop the option to select the output layer name)

@fm3 fm3 changed the title WIP: Allow supplying full workflow yaml for infer_with_model job Allow supplying custom workflow yaml for infer_with_model job Jul 25, 2024
.setScale(2, BigDecimal.RoundingMode.HALF_UP)} seconds and was${if (result.header.status != 200) " not "
else " "}successful"
val debugString =
s"Request ${request.method} ${request.uri} took ${BigDecimal(executionTime.toMillis.toDouble / 1000)
Copy link
Member Author

Choose a reason for hiding this comment

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

slipped in here: the decimals were lost due to integer division

@fm3 fm3 marked this pull request as ready for review July 25, 2024 12:50
@fm3 fm3 requested a review from philippotto July 25, 2024 12:50
@fm3
Copy link
Member Author

fm3 commented Aug 1, 2024

@MichaelBuessemeyer could you please take a look at Philpp’s frontend comments?

@philippotto
Copy link
Member

testing went ok 👍

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

@MichaelBuessemeyer could you please take a look at Philpp’s frontend comments?

Thanks for the ping. Done :)

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

there's a conflict to resolve, but other than that, everything seems good 👍

@fm3 fm3 merged commit 24e1e00 into master Aug 5, 2024
2 checks passed
@fm3 fm3 deleted the inference-full-yaml branch August 5, 2024 06:39
fm3 added a commit that referenced this pull request Aug 5, 2024
philippotto pushed a commit that referenced this pull request Aug 6, 2024
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.

None yet

3 participants