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

User range recommendations; Fixes #177 #178

Merged
merged 8 commits into from
Dec 21, 2016

Conversation

joe4dev
Copy link
Collaborator

@joe4dev joe4dev commented Dec 8, 2016

Add user range support as described in #177
Maintains the existing behavior when triggering single recommendations.

Range generation now separates constructing (i.e., generating the MiniZinc model) from evaluation. See 17a99c0 for more detail.

  • Implement error handling (e.g., recommendation for non-root ingredients, catch exceptions)
  • Remove legacy way of generating deployment recommendations (including seeds!)
  • Test better: particularly check whether the http://localhost:3000/ingredients/:id/recommendations_completed works consistently; check concurrent execution with multiple workers

Dependency: Already rebased on PR #176

@inz inz temporarily deployed to fathomless-escarpment-2-pr-178 December 8, 2016 16:33 Inactive
@dschoeni
Copy link
Contributor

I started implementing the corresponding frontend parts.

GET: /recommendations_completed always returns false though, even if there are no recommendations scheduled anymore. Triggering ranges of recommendations works well however ;)

Separates the construction (generate the MiniZinc model) from the evaluation.
Updating the user workload (i.e., `num_simultaneous_users`),
updating the dependent constraints, and
constructing the MiniZinc model for a certain workload needs to
happen atomically.
Otherwise, concurrent range construction jobs could interfere with each other.
@joe4dev joe4dev force-pushed the feature/user-range-recommendations branch from f3ed913 to 88405f7 Compare December 16, 2016 15:03
@inz inz temporarily deployed to fathomless-escarpment-2-pr-178 December 16, 2016 15:03 Inactive
@joe4dev
Copy link
Collaborator Author

joe4dev commented Dec 16, 2016

Works for me with the latest version and also with some legacy db dumps I migrated for testing purpose.
Also works for @dschoeni now (at least after db reset)

Should be ok but might need some more testing with concurrent executions (as stated in the PR tasks)

@inz inz temporarily deployed to fathomless-escarpment-2-pr-178 December 17, 2016 21:17 Inactive
* Prefer `trigger_range` now
* Including adjusted test cases. These now combine vertical and horizontal scaling.
@inz inz temporarily deployed to fathomless-escarpment-2-pr-178 December 19, 2016 12:33 Inactive
Not needed because the number of simultaneous users is only relevant when
generating new recommendations. Therefore, the number of simultaneous users should not
be statically assigned to an application ingredient (via the user_workload)
@inz inz temporarily deployed to fathomless-escarpment-2-pr-178 December 19, 2016 13:37 Inactive
@joe4dev
Copy link
Collaborator Author

joe4dev commented Dec 19, 2016

Looks ready to merge for me now

@joe4dev joe4dev mentioned this pull request Dec 21, 2016
@dschoeni dschoeni merged commit f0b5421 into master Dec 21, 2016
@joe4dev joe4dev deleted the feature/user-range-recommendations branch December 21, 2016 14:31
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