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

AICORE-381: update model def api #276

Merged
merged 2 commits into from
Nov 9, 2020

Conversation

vpasquier
Copy link
Contributor

No description provided.

@nuxeojenkins
Copy link
Contributor

View issue in JIRA: AICORE-381: Update model definition api

@jcarsique
Copy link
Contributor

jcarsique commented Nov 4, 2020

The server takes too much time to start. There is a new package nuxeo-10.10-HF15-1.0.1 that should be installed in the image before running it.
Trying to fix with #277

@@ -166,13 +165,12 @@ public Response getAllThresholds() {
@Path("extension/model")
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.APPLICATION_JSON)
public Response setModelFromXML(String modelXML) {
public Response setModelFromXML(@QueryParam("modelId") String modelId, String modelXML) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not a PathParam? Or at least there should be a null check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I have no opinion on the query param vs path param but I can change it for all endpoints
  • Thanks for the review, i will check null

@vpasquier vpasquier force-pushed the task-AICORE-381-update-model-def branch 2 times, most recently from 4f08492 to 06017e3 Compare November 4, 2020 22:26
Comment on lines 205 to 207
if(desc==null){

}
if (desc instanceof ThresholdConfiguratorDescriptor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

forgot to remove?

@jcarsique
Copy link
Contributor

The change 20adbd0 to 06017e3 broke testBulkEnrich – org.nuxeo.ai.bulk.BulkEnrichmentTest
https://jenkins.ai.dev.nuxeo.com/blue/organizations/jenkins/nuxeo%2Fnuxeo-ai/detail/PR-276/4/tests
https://github.com/nuxeo/nuxeo-ai/compare/20adbd05695c48e1558ffc0bec55af199b4820da..06017e3

@vpasquier vpasquier force-pushed the task-AICORE-381-update-model-def branch from 1b0d8f2 to f09a77c Compare November 6, 2020 00:07
@nuxeo-ai-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-ai-pr-276 here

@vpasquier vpasquier merged commit 8fa100d into sprint-3a-10.10 Nov 9, 2020
@vpasquier vpasquier deleted the task-AICORE-381-update-model-def branch November 9, 2020 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants