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

Feature/ingest2metrics #58

Merged
merged 8 commits into from
Jan 11, 2024
Merged

Feature/ingest2metrics #58

merged 8 commits into from
Jan 11, 2024

Conversation

deoyani
Copy link
Contributor

@deoyani deoyani commented May 18, 2023

No description provided.

@RayPlante RayPlante self-requested a review December 6, 2023 20:41
Copy link
Collaborator

@RayPlante RayPlante left a comment

Choose a reason for hiding this comment

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

Two issues need correcting:

  • please remove print() statements from python/nistoar/rmm/mongo/nerdm.py. These will not get captured by any log, and they crowd out other useful info in the unit test output.
  • the unit test, python/tests/nistoar/rmm/mongo/test_nerdm.py is failing, and it seems to be for a real reason: the pdrid property is not getting set in the fileMetrices record.

The complaint of the unit test is as follows:

======================================================================
FAIL: test_init_metrics_for (__main__.TestNERDmLoader)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "python/tests/nistoar/rmm/mongo/test_nerdm.py", line 201, in test_init_metrics_for
    self.assertEqual(c[0]['pdrid'], 'ark:/88434/mds2-2106')
AssertionError: None != 'ark:/88434/mds2-2106'

----------------------------------------------------------------------

(To run the unit tests via docker, type scripts/testall.docker. You may need to recreate the test container via docker/dockbuild.sh. See repo's README.md for more information.)

Copy link
Collaborator

@RayPlante RayPlante left a comment

Choose a reason for hiding this comment

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

We have tested this on testdata. Thanks for the updates and the testing!

@RayPlante RayPlante merged commit 5086f5b into integration Jan 11, 2024
2 checks passed
@RayPlante RayPlante deleted the feature/ingest2metrics branch January 11, 2024 14:11
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.

2 participants