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

DataSourceModelAccess.retrieve and related updates #169

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

dschristianson
Copy link
Contributor

@dschristianson dschristianson commented Jul 20, 2023

Update DataSourceModelAccess.retrieve to work properly with the translate layer.

Closes #168

Highlights:

  • Add translate logic to DataSourceModelAccess.retrieve method
  • Removed QueryById query class and added id property to QueryBase
  • Added MonitoringFeatureModelAccess.retrieve class to validate specification of `id'
  • Updated translation logic of query prefixed_fields and validation of prefixed_fields
  • Added tests to explicitly test the retrieve options.
  • Removed use of basin3d_plugin_access decorator and made it private b/c a bug was detected. See basin3d_plugin_access decorator bug #170.

Rationale for removing QueryById:
A particular ModelAccess class (e.g., MonitoringFeatureModelAccess) should use the related query class (e.g. QueryMonitoringFeature). In particular, querying by Monitoring Feature id benefits from having the feature_type available as well.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Unit tests
  • Integration tests
  • Test coverage >= 90%
  • Flake8 Tests
  • Mypy Tests
  • Other - tested with corresponding updates in django-basin3d

Test Configuration

  • Python Version: 3.9

PR Self Evaluation

  • My code follows the agreed upon best practices
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests or modified existing tests that prove my fix is effective or that my feature works
  • Existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in the appropriate modules
  • I have performed a self-review of my own code

@dschristianson dschristianson changed the title DataSourceModelAccess.get update to use translated query and related fixes DRAFT: DataSourceModelAccess.get update to use translated query and related fixes Jul 20, 2023
@dschristianson dschristianson changed the title DRAFT: DataSourceModelAccess.get update to use translated query and related fixes DRAFT: DataSourceModelAccess.retrieve and related updates Jul 20, 2023
Closes #168

Details:
- Add translate logic to DataSourceModelAccess.retrieve method
- Removed QueryById query class and added `id` property to QueryBase
- Added MonitoringFeatureModelAccess.retrieve class to validate specification of `id'
- Updated translation logic of query prefixed_fields and validation of prefixed_fields
- Added tests to explicitly test the retrieve options.
- Removed use of basin3d_plugin_access decorator and made it private
@dschristianson dschristianson changed the title DRAFT: DataSourceModelAccess.retrieve and related updates DataSourceModelAccess.retrieve and related updates Jul 20, 2023
@dschristianson dschristianson marked this pull request as ready for review July 20, 2023 18:45
Copy link
Contributor

@vchendrix vchendrix left a comment

Choose a reason for hiding this comment

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

Looks great. This is an improvement!

@vchendrix vchendrix merged commit a5d16fa into main Jul 20, 2023
@vchendrix vchendrix deleted the issue-168 branch July 20, 2023 20:32
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.

Updates to support django bug
2 participants