Downgrade Play to 2.x to stick to Scala 2.12 across the repo #43
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
project/plugins.sbt (2)
11-14: Approved: Added library dependency scheme for scala-xml.The addition of the library dependency scheme for scala-xml is a good practice to ensure version consistency. The comment provides helpful context by referencing the related GitHub issue.
Consider adding a brief comment explaining the purpose of this setting directly in the code. For example:
// Ensure consistent scala-xml version across the project ThisBuild / libraryDependencySchemes ++= Seq( "org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always )
Line range hint
1-14: Summary: Play Framework downgraded and build settings updated.The changes in this file successfully downgrade the Play Framework plugin to version 2.8.22, which aligns with the PR objective of sticking to Scala 2.12. Additionally, a new build setting has been added to ensure consistent versioning of the scala-xml module.
These changes are crucial for resolving the integration challenges mentioned in the PR objectives. However, it's important to note that downgrading a major framework version may have broader implications on the project:
Compatibility: Some APIs or features available in Play 3.x might not be available or might work differently in Play 2.8.x. A thorough review of the codebase may be necessary to identify and address any incompatibilities.
Performance: There might be performance differences between Play 3.x and 2.8.x. It would be beneficial to run performance tests to ensure that the downgrade doesn't negatively impact the application's performance.
Security: Ensure that Play 2.8.22 doesn't have any known security vulnerabilities. If it does, plan to address them or consider using the latest patch version of Play 2.8.x.
Future Upgrades: As mentioned in the PR objectives, this downgrade is temporary. It would be helpful to create a plan or ticket for the future upgrade to Scala 2.13 and Play 3.x to ensure this technical debt is addressed in the future.
Consider documenting the reasons for this downgrade and the future upgrade plan in a prominent place (e.g., README.md or a separate UPGRADE.md file) to inform other developers and prevent accidental upgrades before the codebase is ready.
hub/test/controllers/ModelControllerSpec.scala (1)
Line range hint
25-54: Consider adding error handling test caseWhile the changes correctly handle successful decoding of the response, it might be beneficial to add a test case that verifies the behavior when decoding fails. This would ensure that the controller properly handles and reports errors in malformed responses.
Would you like me to provide an example of how such a test case could be implemented?
hub/app/controllers/TimeSeriesController.scala (2)
308-308: Approved: Wider range of mock data, but consider documenting the change.The modification to use
random.nextDouble()instead ofrandom.between(0, 0.5)increases the range of generated values from [0, 0.5) to [0, 1). This change provides a wider range of mock data, which could be more representative of real-world scenarios.Consider adding a comment to document the new range of generated values, as this change might affect downstream processing or visualization if those components expect values within a certain range.
313-313: Approved: Consistent change, but document the new range.The modification to use
random.nextDouble()instead ofrandom.between(0, 0.5)is consistent with the change ingenerateMockTimeSeriesPoints. This consistency is good for maintainability.As with the previous change, consider adding a comment to document the new range of generated values (now [0, 1) instead of [0, 0.5)).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- build.sbt (1 hunks)
- docker-init/start.sh (1 hunks)
- hub/app/controllers/TimeSeriesController.scala (1 hunks)
- hub/test/controllers/ModelControllerSpec.scala (2 hunks)
- hub/test/controllers/SearchControllerSpec.scala (2 hunks)
- hub/test/controllers/TimeSeriesControllerSpec.scala (7 hunks)
- project/plugins.sbt (1 hunks)
🧰 Additional context used
🔇 Additional comments (13)
docker-init/start.sh (1)
7-9: Approve the Java options addition with recommendations.The addition of these Java options effectively addresses the
ClassFormatErrorissue. However, I have a few suggestions to improve this change:
Consider adding more detailed documentation about why these specific options are needed. This will help future maintainers understand the context and potential implications.
While this solves the immediate problem, it might be beneficial to investigate the root cause of the module access issue. There might be a more permanent solution that doesn't require opening up module access.
Verify if these options are necessary for all environments or just in the Docker context. If it's specific to Docker, consider moving this to a Docker-specific configuration file.
To ensure this change doesn't introduce any unintended side effects, please run the following verification:
Consider exploring alternative solutions that don't require opening up module access, as this could potentially introduce security vulnerabilities. Some options to consider:
- Upgrade the dependencies that are causing this issue to versions that are compatible with Java 9+ module system.
- If possible, refactor the code to avoid accessing internal Java APIs that are now restricted by the module system.
project/plugins.sbt (1)
10-10: Approved: Play Framework plugin downgraded as intended.The Play Framework plugin has been successfully downgraded from 3.0.5 to 2.8.22, which aligns with the PR objective of sticking to Scala 2.12. The organization change from "org.playframework" to "com.typesafe.play" is correct for Play 2.x versions.
Please ensure that all project dependencies and features are compatible with Play 2.8.22. You may want to run the following command to check for any compatibility issues:
✅ Verification successful
Verified: No deprecated Play Framework APIs detected.
The Play Framework plugin has been successfully downgraded to version 2.8.22 without introducing any deprecated or removed API usages. This ensures compatibility with the project's current codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential compatibility issues with Play 2.8.22 # Search for Play Framework imports and usages echo "Searching for Play Framework imports and usages:" rg --type scala --type java 'import play\.' -A 5 # Search for potential deprecated or removed APIs echo "Searching for potential deprecated or removed APIs:" rg --type scala --type java 'GlobalSettings|Play\.current|Play\.application|Action\.async' # Note: This script provides a starting point. Manual verification is still recommended.Length of output: 4311
hub/test/controllers/ModelControllerSpec.scala (2)
39-39: Correct usage of Either typeThe change from
listModelResponse.value.itemstolistModelResponse.right.value.itemsis correct. This modification properly accesses the Right side of the Either type returned bydecode[ListModelResponse], which contains the successful decoding result. The previous code incorrectly assumed the Either was always in the Left state, which could have led to runtime errors.
51-51: Consistent correction across test casesThis change mirrors the correction made in the previous test case, consistently using
listModelResponse.right.value.itemsto access the Right side of the Either type. This ensures that both the regular and paginated result test cases are handling the decoded response correctly.hub/test/controllers/SearchControllerSpec.scala (2)
39-39: Improved handling of Either typeThe change from
listModelResponse.valuetolistModelResponse.right.valueis a good improvement. It explicitly handles the right side of theEithertype, which represents the success case. This change enhances type safety and makes the code's intent clearer.
51-51: Consistent improvement in Either handlingThe same change has been applied here as in the previous test case, consistently improving the handling of the
Eithertype. This consistency across test cases is commendable as it enhances the overall maintainability and readability of the test suite.build.sbt (1)
214-226: LGTM with suggestions for improvementThe changes to the
hubproject configuration align well with the PR objectives:
- The added comment (lines 214-215) clearly explains the rationale for using Play 2.x, which is helpful for future maintenance.
- The downgrade of
scalatestplus-playto version 5.1.0 (line 222) is consistent with moving to an earlier version of Play framework that supports Scala 2.12.However, there are a couple of points that could be addressed:
The AI summary mentioned changing the Scala version for the
hubproject to 2.12, but this change is not visible in the provided diff. Could you confirm if this change has been made elsewhere or if it needs to be added explicitly to thehubproject settings?It would be beneficial to add a comment specifying the exact Play version being used, as it's currently only referred to as "Play 2.x" in the comment. This would provide more precise information for future reference.
To verify the Scala version setting for the
hubproject, please run the following command:This will display the Scala version configured for the
hubproject. If it's not set to 2.12.18, you may need to add the following line to thehubproject settings:scalaVersion := scala_2_12✅ Verification successful
LGTM with suggestions for improvement
The changes to the
hubproject configuration align well with the PR objectives:
- The added comment (lines 214-215) clearly explains the rationale for using Play 2.x, which is helpful for future maintenance.
- The downgrade of
scalatestplus-playto version 5.1.0 (line 222) is consistent with moving to an earlier version of the Play framework that supports Scala 2.12.- The Scala version for the
hubproject is correctly set to 2.12.18 in the build configuration, ensuring consistency across the repository.- It would be beneficial to add a comment specifying the exact Play version being used, as it's currently only referred to as "Play 2.x" in the comment. This would provide more precise information for future reference.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
sbt "project hub" "show scalaVersion"Length of output: 110
Script:
#!/bin/bash # Search for scalaVersion in build.sbt grep 'scalaVersion' build.sbt # Search for scalaVersion in project/*.sbt grep 'scalaVersion' project/*.sbt # List the contents of the project directory ls project/Length of output: 354
hub/test/controllers/TimeSeriesControllerSpec.scala (6)
48-48: Improved error handling with explicit Either accessThe change from
modelTSResponse.valuetomodelTSResponse.right.valueis a good improvement. It explicitly accesses the right side of the Either type, which represents the success case. This change enhances type safety and makes the code more robust by ensuring that we're only proceeding with the successful case.
90-90: Consistent application of explicit Either accessThis change follows the same pattern as the previous one, using
modelTSResponse.right.valueinstead ofmodelTSResponse.value. It's good to see this consistency maintained throughout the test file, improving error handling and type safety across all test cases.
110-110: Consistent error handling across different test casesThe change to
modelTSResponse.right.valueis consistently applied here as well. It's commendable to see this pattern maintained across different test cases, ensuring uniform and improved error handling throughout the entire test suite.
191-191: Consistent error handling extended to FeatureTimeSeries responsesThe explicit Either access pattern is correctly applied here to the FeatureTimeSeries response. This demonstrates a thorough approach to improving error handling across different response types in the controller. Well done on maintaining consistency throughout the test suite.
210-210: Comprehensive improvement in error handling across all test casesThe consistent application of
*Response.right.valueacross all test cases and response types (including various feature time series lookups) demonstrates a thorough and systematic approach to improving error handling in the entire test suite. This change enhances the robustness and type safety of all tests, making them more resilient to potential errors and easier to maintain.Also applies to: 229-229, 249-249
Line range hint
1-262: Summary: Consistent improvement in error handling aligns with PR objectivesThe changes made to this test file demonstrate a systematic approach to improving error handling and type safety. By consistently using
*Response.right.valueinstead of*Response.value, the code now explicitly handles the success case of Either types. This change:
- Enhances type safety across all test cases.
- Improves error handling without altering the core test logic.
- Makes the tests more robust and easier to maintain.
These modifications align well with the PR objective of downgrading Play to 2.x and sticking to Scala 2.12. By making the code more explicit in its handling of Either types, it ensures compatibility with Scala 2.12 features and idioms. This systematic update across the test suite suggests careful consideration of the implications of the framework downgrade.
Overall, these changes contribute positively to the codebase's quality and maintainability while supporting the transition to an earlier Play version compatible with Scala 2.12.
ken-zlai
left a comment
ken-zlai
left a comment
There was a problem hiding this comment.
Largely looks good, just one point for me to understand about the new number generation
piyush-zlai
force-pushed
the
piyush/play_downgrade
branch
from
8a4b869 to
ebb5ad3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
project/plugins.sbt (1)
11-14: LGTM: Build setting addresses specific issue.The addition of the build setting to specify the version scheme for scala-xml is appropriate:
- It addresses a specific issue (referenced by the GitHub issue link).
- The comment provides context for why this configuration is necessary.
Consider expanding the comment slightly to provide more context:
- // related to: https://github.com/sbt/sbt/issues/6997 + // Addresses version conflicts with scala-xml: https://github.com/sbt/sbt/issues/6997This minor change would make the purpose of the configuration immediately clear to other developers.
build.sbt (1)
214-226: LGTM with a suggestion for explicit Scala version.The changes align well with the PR objective of downgrading Play to 2.x and sticking to Scala 2.12 across the repo. The added comments provide clear reasoning for this decision.
Consider explicitly setting the Scala version for the
hubproject to ensure consistency:lazy val hub = (project in file("hub")) .enablePlugins(PlayScala) .settings( name := "hub", scalaVersion := scala_2_12, // Add this line libraryDependencies ++= Seq( // ... existing dependencies ) )This will make it clear that the
hubproject is using Scala 2.12, consistent with the rest of the repository.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- build.sbt (1 hunks)
- docker-init/start.sh (1 hunks)
- hub/app/controllers/TimeSeriesController.scala (1 hunks)
- hub/test/controllers/ModelControllerSpec.scala (2 hunks)
- hub/test/controllers/SearchControllerSpec.scala (2 hunks)
- hub/test/controllers/TimeSeriesControllerSpec.scala (7 hunks)
- project/plugins.sbt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- docker-init/start.sh
- hub/app/controllers/TimeSeriesController.scala
- hub/test/controllers/ModelControllerSpec.scala
- hub/test/controllers/SearchControllerSpec.scala
- hub/test/controllers/TimeSeriesControllerSpec.scala
🧰 Additional context used
🔇 Additional comments (2)
project/plugins.sbt (1)
Line range hint
1-10: LGTM: Plugin additions and Play Framework downgrade align with PR objectives.The changes to the plugins are appropriate:
- The addition of various SBT plugins (sbt-assembly, sbt-scalafmt, sbt-pgp, sbt-release, sbt-git, sbt-buildinfo, sbt-shading, sbt-scalafix) enhances the project's build and development capabilities.
- The downgrade of the Play Framework plugin from 3.0.5 to 2.8.22 aligns with the PR objective to stick to Scala 2.12 across the repo.
- The addition of the dependency tree plugin will help in managing complex dependency structures.
These changes support the goal of maintaining compatibility with Scala 2.12 while improving the build process.
build.sbt (1)
Line range hint
1-238: Verify Scala versions across all modules.While the changes for the
hubproject look good, it's important to ensure that all modules in the project are consistently using Scala 2.12 as intended by this PR.Let's run a quick check to verify the Scala versions used across all modules:
This will help us confirm that all modules are set to use Scala 2.12 and identify any modules that might need adjustment.
✅ Verification successful
Scala Versions Consistency Verified
All modules are consistently using Scala 2.12 as intended by this PR.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash echo "Checking Scala versions across all modules:" grep -n "scalaVersion :=" build.sbt echo "Checking crossScalaVersions:" grep -n "crossScalaVersions :=" build.sbtLength of output: 418
## Summary PR overview - [Video Link](https://drive.google.com/file/d/1Rei6upL2OiAls2jX7mCERaJGai3Noo1j/view?usp=drive_link) This PR builds on #33 and #43. We register the relevant model / join / groupby / staging query artifacts at the 'app' docker container startup by using the MetadataUploader and hitting the Dynamo endpoints using the KV store. We also extend the API to stop returning mocked data for the list and search calls and start returning real registered models + enriched responses (so the model object includes details on the Joins, GroupBys and features). There were a few broken pieces along the way that I fixed while working through the integration (e.g. the metadata walker code was missing handling models, the api.thrift enum for model type needed to start at index 0 etc). ## Checklist - [X] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update Bringing up the container and curling: ``` $ curl http://localhost:9000/api/v1/search?term=1&limit=20 {"offset":0,"items":[{"name":"risk.transaction_model.v1","join":{"name":"risk.user_transactions.txn_join","joinFeatures":[],"groupBys":[{"name":"risk.transaction_events.txn_group_by_user","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.transaction_events.txn_group_by_merchant","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.user_data.user_group_by","features":["account_age","account_balance","credit_score","number_of_devices","country","account_type","preferred_language"]},{"name":"risk.merchant_data.merchant_group_by","features":["account_age","zipcode","is_big_merchant","country","account_type","preferred_language"]}]},"online":false,"production":false,"team":"risk","modelType":"XGBoost"}]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `DynamoDBMonitoringStore` for improved model data retrieval. - Enhanced `Model` structure to include joins and groupings for better data organization. - Expanded `parseTeam` method to handle `Model` instances in metadata processing. - Updated `ModelController` and `SearchController` to utilize real data from `DynamoDBMonitoringStore`. - Introduced new methods for managing datasets in the `MetadataStore`. - Improved handling of list values in the DynamoDB key-value store implementation. - Added support for AWS services in the application through new Docker configurations. - Enhanced the application's Docker setup for better build and runtime environments. - Modified the application's metadata loading process to include a new section for DynamoDB. - **Bug Fixes** - Corrected handling of `Either` types in test cases to prevent runtime errors. - **Documentation** - Updated configuration files to support new DynamoDB module. - **Tests** - Added comprehensive unit tests for `DynamoDBKVStoreImpl` and `DynamoDBMonitoringStore`. - Enhanced test coverage for `ModelController` and `SearchController` with mocked dependencies. - Introduced new tests for the functionality of `DynamoDBMonitoringStore`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Downgrade Play to 2.x. The version of Play we had required Scala 2.13. This ends up being painful to work with when we start introducing dependencies from the 'hub' play module to other modules like api / online (which we need for the Dynamodb hookup). Sbt isn't very happy when we're trying to hook up these builds across scala versions when hub is defaulting to 2.13 and the rest of the project on 2.12. To not bang at this further, I'm downgrading to a play version on scala 2.12 and we can revisit when we're ready to bump to Scala 2.13. ## Checklist - [ ] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced mock data generation for time series, allowing for a broader range of generated values. - Updated Java options in the Docker initialization script to prevent execution errors. - **Bug Fixes** - Improved error handling in test cases by correctly accessing the successful results from decoded responses. - **Chores** - Updated project dependencies and plugins to maintain compatibility and improve build configuration, including the Play Framework plugin and several SBT plugins. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary PR overview - [Video Link](https://drive.google.com/file/d/1Rei6upL2OiAls2jX7mCERaJGai3Noo1j/view?usp=drive_link) This PR builds on #33 and #43. We register the relevant model / join / groupby / staging query artifacts at the 'app' docker container startup by using the MetadataUploader and hitting the Dynamo endpoints using the KV store. We also extend the API to stop returning mocked data for the list and search calls and start returning real registered models + enriched responses (so the model object includes details on the Joins, GroupBys and features). There were a few broken pieces along the way that I fixed while working through the integration (e.g. the metadata walker code was missing handling models, the api.thrift enum for model type needed to start at index 0 etc). ## Checklist - [X] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update Bringing up the container and curling: ``` $ curl http://localhost:9000/api/v1/search?term=1&limit=20 {"offset":0,"items":[{"name":"risk.transaction_model.v1","join":{"name":"risk.user_transactions.txn_join","joinFeatures":[],"groupBys":[{"name":"risk.transaction_events.txn_group_by_user","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.transaction_events.txn_group_by_merchant","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.user_data.user_group_by","features":["account_age","account_balance","credit_score","number_of_devices","country","account_type","preferred_language"]},{"name":"risk.merchant_data.merchant_group_by","features":["account_age","zipcode","is_big_merchant","country","account_type","preferred_language"]}]},"online":false,"production":false,"team":"risk","modelType":"XGBoost"}]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `DynamoDBMonitoringStore` for improved model data retrieval. - Enhanced `Model` structure to include joins and groupings for better data organization. - Expanded `parseTeam` method to handle `Model` instances in metadata processing. - Updated `ModelController` and `SearchController` to utilize real data from `DynamoDBMonitoringStore`. - Introduced new methods for managing datasets in the `MetadataStore`. - Improved handling of list values in the DynamoDB key-value store implementation. - Added support for AWS services in the application through new Docker configurations. - Enhanced the application's Docker setup for better build and runtime environments. - Modified the application's metadata loading process to include a new section for DynamoDB. - **Bug Fixes** - Corrected handling of `Either` types in test cases to prevent runtime errors. - **Documentation** - Updated configuration files to support new DynamoDB module. - **Tests** - Added comprehensive unit tests for `DynamoDBKVStoreImpl` and `DynamoDBMonitoringStore`. - Enhanced test coverage for `ModelController` and `SearchController` with mocked dependencies. - Introduced new tests for the functionality of `DynamoDBMonitoringStore`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Downgrade Play to 2.x. The version of Play we had required Scala 2.13. This ends up being painful to work with when we start introducing dependencies from the 'hub' play module to other modules like api / online (which we need for the Dynamodb hookup). Sbt isn't very happy when we're trying to hook up these builds across scala versions when hub is defaulting to 2.13 and the rest of the project on 2.12. To not bang at this further, I'm downgrading to a play version on scala 2.12 and we can revisit when we're ready to bump to Scala 2.13. ## Checklist - [ ] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced mock data generation for time series, allowing for a broader range of generated values. - Updated Java options in the Docker initialization script to prevent execution errors. - **Bug Fixes** - Improved error handling in test cases by correctly accessing the successful results from decoded responses. - **Chores** - Updated project dependencies and plugins to maintain compatibility and improve build configuration, including the Play Framework plugin and several SBT plugins. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary PR overview - [Video Link](https://drive.google.com/file/d/1Rei6upL2OiAls2jX7mCERaJGai3Noo1j/view?usp=drive_link) This PR builds on #33 and #43. We register the relevant model / join / groupby / staging query artifacts at the 'app' docker container startup by using the MetadataUploader and hitting the Dynamo endpoints using the KV store. We also extend the API to stop returning mocked data for the list and search calls and start returning real registered models + enriched responses (so the model object includes details on the Joins, GroupBys and features). There were a few broken pieces along the way that I fixed while working through the integration (e.g. the metadata walker code was missing handling models, the api.thrift enum for model type needed to start at index 0 etc). ## Checklist - [X] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update Bringing up the container and curling: ``` $ curl http://localhost:9000/api/v1/search?term=1&limit=20 {"offset":0,"items":[{"name":"risk.transaction_model.v1","join":{"name":"risk.user_transactions.txn_join","joinFeatures":[],"groupBys":[{"name":"risk.transaction_events.txn_group_by_user","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.transaction_events.txn_group_by_merchant","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.user_data.user_group_by","features":["account_age","account_balance","credit_score","number_of_devices","country","account_type","preferred_language"]},{"name":"risk.merchant_data.merchant_group_by","features":["account_age","zipcode","is_big_merchant","country","account_type","preferred_language"]}]},"online":false,"production":false,"team":"risk","modelType":"XGBoost"}]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `DynamoDBMonitoringStore` for improved model data retrieval. - Enhanced `Model` structure to include joins and groupings for better data organization. - Expanded `parseTeam` method to handle `Model` instances in metadata processing. - Updated `ModelController` and `SearchController` to utilize real data from `DynamoDBMonitoringStore`. - Introduced new methods for managing datasets in the `MetadataStore`. - Improved handling of list values in the DynamoDB key-value store implementation. - Added support for AWS services in the application through new Docker configurations. - Enhanced the application's Docker setup for better build and runtime environments. - Modified the application's metadata loading process to include a new section for DynamoDB. - **Bug Fixes** - Corrected handling of `Either` types in test cases to prevent runtime errors. - **Documentation** - Updated configuration files to support new DynamoDB module. - **Tests** - Added comprehensive unit tests for `DynamoDBKVStoreImpl` and `DynamoDBMonitoringStore`. - Enhanced test coverage for `ModelController` and `SearchController` with mocked dependencies. - Introduced new tests for the functionality of `DynamoDBMonitoringStore`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Downgrade Play to 2.x. The version of Play we had required Scala 2.13. This ends up being painful to work with when we start introducing dependencies from the 'hub' play module to other modules like api / online (which we need for the Dynamodb hookup). Sbt isn't very happy when we're trying to hook up these builds across scala versions when hub is defaulting to 2.13 and the rest of the project on 2.12. To not bang at this further, I'm downgrading to a play version on scala 2.12 and we can revisit when we're ready to bump to Scala 2.13. ## Checklist - [ ] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced mock data generation for time series, allowing for a broader range of generated values. - Updated Java options in the Docker initialization script to prevent execution errors. - **Bug Fixes** - Improved error handling in test cases by correctly accessing the successful results from decoded responses. - **Chores** - Updated project dependencies and plugins to maintain compatibility and improve build configuration, including the Play Framework plugin and several SBT plugins. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary PR overview - [Video Link](https://drive.google.com/file/d/1Rei6upL2OiAls2jX7mCERaJGai3Noo1j/view?usp=drive_link) This PR builds on #33 and #43. We register the relevant model / join / groupby / staging query artifacts at the 'app' docker container startup by using the MetadataUploader and hitting the Dynamo endpoints using the KV store. We also extend the API to stop returning mocked data for the list and search calls and start returning real registered models + enriched responses (so the model object includes details on the Joins, GroupBys and features). There were a few broken pieces along the way that I fixed while working through the integration (e.g. the metadata walker code was missing handling models, the api.thrift enum for model type needed to start at index 0 etc). ## Checklist - [X] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update Bringing up the container and curling: ``` $ curl http://localhost:9000/api/v1/search?term=1&limit=20 {"offset":0,"items":[{"name":"risk.transaction_model.v1","join":{"name":"risk.user_transactions.txn_join","joinFeatures":[],"groupBys":[{"name":"risk.transaction_events.txn_group_by_user","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.transaction_events.txn_group_by_merchant","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.user_data.user_group_by","features":["account_age","account_balance","credit_score","number_of_devices","country","account_type","preferred_language"]},{"name":"risk.merchant_data.merchant_group_by","features":["account_age","zipcode","is_big_merchant","country","account_type","preferred_language"]}]},"online":false,"production":false,"team":"risk","modelType":"XGBoost"}]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `DynamoDBMonitoringStore` for improved model data retrieval. - Enhanced `Model` structure to include joins and groupings for better data organization. - Expanded `parseTeam` method to handle `Model` instances in metadata processing. - Updated `ModelController` and `SearchController` to utilize real data from `DynamoDBMonitoringStore`. - Introduced new methods for managing datasets in the `MetadataStore`. - Improved handling of list values in the DynamoDB key-value store implementation. - Added support for AWS services in the application through new Docker configurations. - Enhanced the application's Docker setup for better build and runtime environments. - Modified the application's metadata loading process to include a new section for DynamoDB. - **Bug Fixes** - Corrected handling of `Either` types in test cases to prevent runtime errors. - **Documentation** - Updated configuration files to support new DynamoDB module. - **Tests** - Added comprehensive unit tests for `DynamoDBKVStoreImpl` and `DynamoDBMonitoringStore`. - Enhanced test coverage for `ModelController` and `SearchController` with mocked dependencies. - Introduced new tests for the functionality of `DynamoDBMonitoringStore`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…#43) ## Summary Downgrade Play to 2.x. The version of Play we had required Scala 2.13. This ends up being painful to work with when we start introducing dependencies from the 'hub' play module to other modules like api / online (which we need for the Dynamodb hookup). Sbt isn't very happy when we're trying to hook up these builds across scala versions when hub is defaulting to 2.13 and the rest of the project on 2.12. To not bang at this further, I'm downgrading to a play version on scala 2.12 and we can revisit when we're ready to bump to Scala 2.13. ## Cheour clientslist - [ ] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced moour clients data generation for time series, allowing for a broader range of generated values. - Updated Java options in the Doour clientser initialization script to prevent execution errors. - **Bug Fixes** - Improved error handling in test cases by correctly accessing the successful results from decoded responses. - **Chores** - Updated project dependencies and plugins to maintain compatibility and improve build configuration, including the Play Framework plugin and several SBT plugins. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary PR overview - [Video Link](https://drive.google.com/file/d/1Rei6upL2OiAls2jX7mCERaJGai3Noo1j/view?usp=drive_link) This PR builds on #33 and #43. We register the relevant model / join / groupby / staging query artifacts at the 'app' doour clientser container startup by using the MetadataUploader and hitting the Dynamo endpoints using the KV store. We also extend the API to stop returning moour clientsed data for the list and search calls and start returning real registered models + enriched responses (so the model object includes details on the Joins, GroupBys and features). There were a few broken pieces along the way that I fixed while working through the integration (e.g. the metadata walker code was missing handling models, the api.thrift enum for model type needed to start at index 0 etc). ## Cheour clientslist - [X] Added Unit Tests - [X] Covered by existing CI - [X] Integration tested - [ ] Documentation update Bringing up the container and curling: ``` $ curl http://localhost:9000/api/v1/search?term=1&limit=20 {"offset":0,"items":[{"name":"risk.transaction_model.v1","join":{"name":"risk.user_transactions.txn_join","joinFeatures":[],"groupBys":[{"name":"risk.transaction_events.txn_group_by_user","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.transaction_events.txn_group_by_merchant","features":["transaction_amount_count_1h","transaction_amount_count_1d","transaction_amount_count_30d","transaction_amount_count_365d","transaction_amount_sum_1h"]},{"name":"risk.user_data.user_group_by","features":["account_age","account_balance","credit_score","number_of_devices","country","account_type","preferred_language"]},{"name":"risk.merchant_data.merchant_group_by","features":["account_age","zipcode","is_big_merchant","country","account_type","preferred_language"]}]},"online":false,"production":false,"team":"risk","modelType":"XGBoost"}]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `DynamoDBMonitoringStore` for improved model data retrieval. - Enhanced `Model` structure to include joins and groupings for better data organization. - Expanded `parseTeam` method to handle `Model` instances in metadata processing. - Updated `ModelController` and `SearchController` to utilize real data from `DynamoDBMonitoringStore`. - Introduced new methods for managing datasets in the `MetadataStore`. - Improved handling of list values in the DynamoDB key-value store implementation. - Added support for AWS services in the application through new Doour clientser configurations. - Enhanced the application's Doour clientser setup for better build and runtime environments. - Modified the application's metadata loading process to include a new section for DynamoDB. - **Bug Fixes** - Corrected handling of `Either` types in test cases to prevent runtime errors. - **Documentation** - Updated configuration files to support new DynamoDB module. - **Tests** - Added comprehensive unit tests for `DynamoDBKVStoreImpl` and `DynamoDBMonitoringStore`. - Enhanced test coverage for `ModelController` and `SearchController` with moour clientsed dependencies. - Introduced new tests for the functionality of `DynamoDBMonitoringStore`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Downgrade Play to 2.x. The version of Play we had required Scala 2.13. This ends up being painful to work with when we start introducing dependencies from the 'hub' play module to other modules like api / online (which we need for the Dynamodb hookup). Sbt isn't very happy when we're trying to hook up these builds across scala versions when hub is defaulting to 2.13 and the rest of the project on 2.12. To not bang at this further, I'm downgrading to a play version on scala 2.12 and we can revisit when we're ready to bump to Scala 2.13.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores