Skip to content

Additional Endpoint Documentation#41

Merged
schweikart merged 18 commits intodevelopfrom
docs/endpoint-documentation
Sep 28, 2023
Merged

Additional Endpoint Documentation#41
schweikart merged 18 commits intodevelopfrom
docs/endpoint-documentation

Conversation

@Elscrux
Copy link
Copy Markdown
Member

@Elscrux Elscrux commented Sep 3, 2023

Initial draft to get automatic documentation for our endpoints in place. A review would already be good at this point.
There are some things that might need a better approach.
For example, the way I convert objects to JSON for the documentation is quite messy with the required try/catch.
Also, there could definitely be more description texts to explain the API in more detail.
I'll probably add descriptions to all the parameters you pass in the future.

I also reworked how arguments are passed into the api tests. They should now test all solvers automatically and they should now use the examples from the documentation. The way I setup the examples was by adding a method to MetaSolver which can be overridden by all individual meta solvers. Do you think that's a good way to do it or would you prefer a more decoupled way?

@Elscrux Elscrux requested a review from schweikart September 3, 2023 15:02
@Elscrux Elscrux changed the base branch from refactor/feature-model-anomaly to develop September 8, 2023 20:20
@Elscrux Elscrux force-pushed the docs/endpoint-documentation branch 2 times, most recently from 0ecc1ef to a48baac Compare September 8, 2023 20:46
@schweikart schweikart added the type: documentation Improvements or additions to documentation label Sep 10, 2023
Comment thread src/main/java/edu/kit/provideq/toolbox/api/MetaSolverSettingsRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/maxcut/MetaSolverMaxCut.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SolveRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SolversRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SubRoutineRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SubRoutineRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SubRoutineRouter.java Outdated
@Elscrux Elscrux force-pushed the docs/endpoint-documentation branch from 1fae88b to f6ac2cf Compare September 11, 2023 13:37
@Elscrux
Copy link
Copy Markdown
Member Author

Elscrux commented Sep 11, 2023

@schweikart I've fixed pretty much everything you asked for, some points might need some discussion.
Can you review the changes I made for this? Especially concerning putting everything into the resources. Basically, it works the way that every problem type gets a folder in which they can store files with examples. Getting those examples is currently implemented in the special meta solvers everywhere and they are pretty much the same as they all use String problem input. But in case a problem wouldn't be of type string we'd need something else for this. That is why I didn't generalize this, but maybe we can improve this still.

The tests are still failing because #44 needs to be merged in this first to fix the usage of the type in SubRoutineDefinition

Copy link
Copy Markdown
Member

@schweikart schweikart left a comment

Choose a reason for hiding this comment

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

Most things work nicely now but I have a few more suggestions and comments

Comment thread src/test/java/edu/kit/provideq/toolbox/api/MaxCutSolversTest.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SolveRouter.java
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SubRoutineRouter.java Outdated
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SolveRouter.java
Comment thread src/main/java/edu/kit/provideq/toolbox/api/SolveRouter.java Outdated
@schweikart
Copy link
Copy Markdown
Member

Our request examples always show "requestContent": null in each sub-solve request since we're using the same object for both our requests and our sub-solve-requests. We never use this attribute so it's kind of useless that we allow (and document) it. Should we maybe use another type for sub-solve-requests? Maybe a new SolverChoice as a supertype of SolveRequest?

{
  "requestContent": "namespace Sandwich\n\nfeatures\n    Sandwich {extended__}\n        mandatory\n            Bread\n                alternative\n                    \"Full Grain\" {Calories 203, Price 1.99, Organic true}\n                    Flatbread {Calories 90, Price 0.79, Organic true}\n                    Toast {Calories 250, Price 0.99, Organic false}\n        optional\n            Cheese\n                optional\n                    Gouda\n                        alternative\n                            Sprinkled {Fat {value 35, unit \"g\"}}\n                            Slice {Fat {value 35, unit \"g\"}}\n                    Cheddar\n                    \"Cream Cheese\"\n            Meat\n                or\n                    \"Salami\" {Producer \"Farmer Bob\"}\n                    Ham {Producer \"Farmer Sam\"}\n                    \"Chicken Breast\" {Producer \"Farmer Sam\"}\n            Vegetables\n                optional\n                    \"Cucumber\"\n                    Tomatoes\n                    Lettuce",
  "requestedSolverId": "edu.kit.provideq.toolbox.featuremodel.anomaly.dead.SatBasedDeadFeatureSolver",
  "requestedMetaSolverSettings": [],
  "requestedSubSolveRequests": {
    "sat": {
      "requestContent": null,
      "requestedSolverId": "edu.kit.provideq.toolbox.sat.solvers.GamsSatSolver",
      "requestedMetaSolverSettings": null,
      "requestedSubSolveRequests": null
    }
  }
}

We shouldn't work on this in this PR but do you agree that we should change this?

@Elscrux Elscrux mentioned this pull request Sep 14, 2023
@Elscrux Elscrux force-pushed the docs/endpoint-documentation branch from 4ef05a1 to 2391671 Compare September 18, 2023 16:35
@Elscrux
Copy link
Copy Markdown
Member Author

Elscrux commented Sep 18, 2023

I rebased onto dev in order to merge the changes of the Cirq MaxCut Solver

@schweikart schweikart force-pushed the docs/endpoint-documentation branch from 2391671 to cddff4c Compare September 28, 2023 14:55
Copy link
Copy Markdown
Member

@schweikart schweikart left a comment

Choose a reason for hiding this comment

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

I've checked everything you've recently addressed, rebased and tried it out on my machine. Seems like this is finally ready to merge!

@schweikart schweikart merged commit 69233c9 into develop Sep 28, 2023
@schweikart schweikart mentioned this pull request Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants