Skip to content

Finished - #1

Open
Kostaga wants to merge 6 commits into
pantelis17:mainfrom
Kostaga:demo
Open

Finished#1
Kostaga wants to merge 6 commits into
pantelis17:mainfrom
Kostaga:demo

Conversation

@Kostaga

@Kostaga Kostaga commented Jun 20, 2025

Copy link
Copy Markdown

No description provided.

@pantelis17

Copy link
Copy Markdown
Owner

Code Review Suggestions

Docker / Deployment

  • Move all Docker Compose files into a dedicated directory (e.g. docker/) to keep the main project directory clean.
  • Add a README.md inside that directory explaining the purpose of each Compose file.
  • Use a .env file to centralize shared parameters (such as image versions), avoiding duplication across Compose files.
  • Consider using a base Compose file approach:
    docker-compose -f docker-compose-base.yml -f docker-compose.node10.yml up -d
    
    This allows common fields to reside in the base file, with node-specific files only overriding necessary differences (like hostname, ports, HOSTNAME, DISCOVERY). This reduces each node file to just 3-4 fields.

Kafka

General Cleanup

  • Remove tempCodeRunnerFile.py since it does not appear to be used.

Python

insertData.py

  • In generate_metrics, avoid using random for higher_is_better; it should be determined by entity_type.
  • In save_sql, remove the icon from the log message for clarity.

json_clean/test.py

  • Rename the file to a more descriptive name.
  • Use variables for the metric and suffix lists to improve readability.

nulls/compute_nulls.py

  • Extract the hard-coded JSON into a separate file, similar to the approach in json_clean.

Java

JoinController.java

  • Move the main logic from the getJoinedMetrics endpoint into JoinService.
  • Within JoinService, delegate to MetricService for retrieving metrics.
  • Remove direct use of MetricRepository in the controller.
  • Clean up extra empty lines at the end of the class.

ContainerEntity.java

  • Rename fields to follow Java naming conventions (camelCase).
  • If JSON requires a different format (for example location_y), use:
    @JsonProperty("location_y")
    private float locationY;
    

MetricRepository.java

  • Remove the unused import for Param.

JoinService.java

  • Prefer final var (or var) for local variables to improve readability.
  • Rename threshold to TUPLE_THRESHOLD_RATIO and make it static final since its value does not change.
  • Make DhtService a final field as well.
  • Replace System.out.println statements with proper logging. If there are issues with the logger configuration, we can resolve them together.
  • Avoid returning raw Object types. For example, groupByContainerAndTimestamp should return a specific type.
  • Wrap the for loop content in groupByContainerAndTimestamp in a try-catch block to handle possible parsing exceptions.
  • Consider splitting functionality inside that for loop into separate methods for clarity. For instance, move the copyIfAbsent block to a method that accepts a list of strings and performs the check.
  • Use standard JavaDoc comments (/** ... */) for method documentation.
  • In joinLocally, instead of manually populating a Map<String, Object>, use DTOs such as MetricDTO, PodMetricDTO, and ContainerMetricDTO with constructors. This improves readability and maintainability.

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