Skip to content

ComputationResources Method Implementations for REST API#4

Draft
zack-rma wants to merge 20 commits into
adamkorynta:devops/refactor-integration-fixturesfrom
zack-rma:devops/refactor-integration-fixtures_computation_resources
Draft

ComputationResources Method Implementations for REST API#4
zack-rma wants to merge 20 commits into
adamkorynta:devops/refactor-integration-fixturesfrom
zack-rma:devops/refactor-integration-fixtures_computation_resources

Conversation

@zack-rma

@zack-rma zack-rma commented Jan 7, 2025

Copy link
Copy Markdown

Problem Description

Fixes #221.

Solution

Implemented new methods to support reference retrieval filtering.

how you tested the change

Tested with REST API against OpenTSDB.

Where the following done:

  • Tests. Check all that apply:
    • Unit tests created or modified that run during ant test.
    • Integration tests created or modified that run during integration testing
      (Formerly called regression tests.)
    • Test procedure descriptions for manual testing
  • Was relevant documentation updated?
  • Were relevant config element (e.g. XML data) updated as appropriate

If you aren't sure leave unchecked and we will help guide you to want needs changing where.

@zack-rma zack-rma changed the base branch from unused to devops/refactor-integration-fixtures January 7, 2025 01:07

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Definitely a solid idea with the CompRefFilter, but it should derive (in the general and literal sense) from the standard mechanisms of Java.

* @param filter the computation filter containing app and algorithm IDs
* @return List of computations
*/
List<DbComputation> listCompRefsForREST(CompRefFilter filter)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like this should really just take a Predicate, you're implementation above should then derive from Predicate

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also we can just name this listCompRefsMatching. This is also useful outside the REST API.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed and added predicates to the filter class

List<DbComputation> ret = new ArrayList<>();
for (DbObjectCache<DbComputation>.CacheIterator it = compCache.iterator(); it.hasNext(); )
{
DbComputation comp = it.next();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah, it seems like this loop should just take the predicate and use it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Simplified to use predicate from filter class

@adamkorynta adamkorynta force-pushed the devops/refactor-integration-fixtures branch from 500efb2 to d9cdd7f Compare January 22, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants