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.

Sign up for GitHub

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

Jump to bottom

feat(DCF-450): Add DRS Fields #357

Merged
merged 7 commits into from
Apr 25, 2023
Merged

feat(DCF-450): Add DRS Fields #357

merged 7 commits into from
Apr 25, 2023

Conversation

k-burt-uch
Copy link
Contributor

@k-burt-uch k-burt-uch commented Apr 10, 2023 •
edited by paulineribeyre
Loading

Jira Ticket: DCF-450

New Features

  • Adds description, content_updated_time and content_created_time to IndexRecord.
  • DRS responses now map updated_time and created_time to the underlying content's updated and created times.
  • DRS responses now include index_created_time and index_updated_time to denote the time they were indexed and the time their index record was changed. These fields replace the previous updated_time and created_time in older indexd versions.

Breaking Changes

  • DRS responses for update_time and created_time reflect the content's updated and created times, not the time they were indexed or their index record was updated.

Bug Fixes

Improvements

Dependency updates

Deployment changes

  • New columns added to index_record table. Alembic migration needed.

@k-burt-uch k-burt-uch requested a review from BinamB April 10, 2023 21:34
@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@k-burt-uch k-burt-uch force-pushed the feat/DCF-450-Drs-description-and-timestamps branch from c506ec7 to 2764cbc Compare April 19, 2023 18:07
@k-burt-uch
feat(DCF-450): Add DRS Fields
1845dd8 
Adds description, content_updated_time and content_created_time to IndexRecord.

Maps content_updated_time and content_created_time to updated_time and
created_time in the DRS responses.

Maps IndexRecord updated_time and created_time to index_updated_time and
index_created_time in DRS responses.
@k-burt-uch k-burt-uch force-pushed the feat/DCF-450-Drs-description-and-timestamps branch from 2764cbc to 1845dd8 Compare April 19, 2023 18:15
@k-burt-uch k-burt-uch marked this pull request as ready for review April 19, 2023 18:26
paulineribeyre
paulineribeyre reviewed Apr 19, 2023
View reviewed changes
migrations/versions/a72f117515c5_add_description_created_time_and_.py Outdated Show resolved Hide resolved
indexd/drs/blueprint.py Outdated Show resolved Hide resolved
indexd/index/blueprint.py Outdated Show resolved Hide resolved
tests/postgres/migrations/test_a72f117515c5_add_description_created_time_and_.py Outdated Show resolved Hide resolved
tests/test_driver_alchemy_crud.py Show resolved Hide resolved
indexd/index/drivers/alchemy.py Show resolved Hide resolved
tests/test_drs.py Outdated Show resolved Hide resolved
tests/test_drs.py Show resolved Hide resolved
tests/test_drs.py
Comment on lines +61 to +63
bundle = get_bundle_doc(bundles=dids)
if has_description:
bundle["description"] = "A description"
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe get_bundle_doc should just accept a description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason we'd need to check an inputted description rather than just a generic string here?

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not sure i understand your question, i meant that you could do bundle = get_bundle_doc(bundles=dids, has_description=True) or bundle = get_bundle_doc(bundles=dids, description=xxx) on L61 and let get_bundle_doc add the description

but it's just a detail

@k-burt-uch k-burt-uch force-pushed the feat/DCF-450-Drs-description-and-timestamps branch from 0c274e1 to db66756 Compare April 20, 2023 23:09
BinamB
BinamB reviewed Apr 24, 2023
View reviewed changes
tests/test_client.py
@@ -2646,3 +2646,56 @@ def test_get_dist(client):
"type": "indexd",
}
]


def test_changing_timestamps_updated_not_before_created(client, user):
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add doc string to these tests? ex: "test that the updated date entered does not come before created date"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to the confusingly named ones, let me know if there's more docs you want me to add for these tests.

@k-burt-uch k-burt-uch merged commit 6cf4627 into master Apr 25, 2023
@k-burt-uch k-burt-uch deleted the feat/DCF-450-Drs-description-and-timestamps branch April 25, 2023 17:52
This was referenced Jun 6, 2023
Open
Merged
Merged
Merged
Merged
Merged
This was referenced Sep 12, 2023
Closed
Closed
Closed
Merged
Closed
Merged
Open
This was referenced Oct 3, 2023
Merged
Merged
Merged
Closed
Closed
Closed
@github-actions github-actions bot mentioned this pull request Oct 10, 2023
Open
@github-actions github-actions bot mentioned this pull request Oct 26, 2023
Merged
This was referenced Nov 2, 2023
Merged
Merged
Merged
Merged
@github-actions github-actions bot mentioned this pull request Nov 20, 2023
Open
@github-actions github-actions bot mentioned this pull request Nov 30, 2023
Merged
This was referenced Dec 11, 2023
Closed
Merged
Closed
Merged
Open
This was referenced Dec 19, 2023
Open
Open
Open
@github-actions github-actions bot mentioned this pull request Feb 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BinamB BinamB BinamB approved these changes

@paulineribeyre paulineribeyre paulineribeyre approved these changes

Assignees
No one assigned
Labels
test-apis-etlTest test-coremetadata-getCoremetadataTest test-portal-coremetadatapageTest test-portal-dataUploadTest
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@k-burt-uch @paulineribeyre @BinamB @PlanXCyborg