Tdl 13966 update custom fields#36
Conversation
|
|
||
| # verify custom field schema | ||
| if stream in self.custom_command_streams: | ||
| actual_custom_field_keys = list(schema["properties"] |
There was a problem hiding this comment.
We should be testing this in metadata not annotated-schema. The menagerie method call is misleadingly named. But annotated-schema is no longer used by our front-end, only metadata is. You should be able to make the same assertion just change the way the actual value is accessed.
There was a problem hiding this comment.
@kspeer825 This test is validating nested fields present under the CustomField field.
However, as metadata only contains first-level fields, can you please help us on how to retrieve nested fields from metadata?
There was a problem hiding this comment.
I am not sure this is possible. The metadata is what drives table and field selection in our ui. And nested field selection is not a feature that I am aware of, so I think this would require a change to singer as well as our frontend. I will confirm this with a developer and get back to you.
There was a problem hiding this comment.
I confirmed this with the devs. I think in this test discovering the top level key from metadata is sufficient. If you need to confirm that the subfields are picked up by the tap, you would need a test that performs a sync and you could pull the actual field values as well as the json schema from the messages sent to the target.
There was a problem hiding this comment.
@kspeer825, Updated test, and accessed top-level field value from metadata instead of annotated-schema. The subfields validation from sync is already added as part of the test_quickbooks_sync_all.py test.
NOTE: The CircleCI build is failing currently due to dependency installation failure which is observed in some taps from last week.
kspeer825
left a comment
kspeer825
left a comment
There was a problem hiding this comment.
Tests look good. For other taps experiencing this dependency issue, we have unpinned ipdb in setup and split up the install groups into a dev and `test. Here is an example of the change singer-io/tap-google-analytics@a7cbd77 .
Updated setup to resolve dependency issue as per above reference. |
savan-chovatiya
requested review from
luandy64 and
zachharris1
and removed request for
KAllan357
* TDL-13967: Added ProfitAndLoss report stream * TDL-13967: Handled state updation if no records found * Added comments in unittests * Updated sync function for report streams * Fixed integration tests * Added stream into readme * Break down report parser for rows and columns * Resolved circleci failure * Updated date retrival from datetime object * Break down dev and test ddependancy in setup * TDL-13967: Resolved review comments * TDL-13967: Added code comments Co-authored-by: savan-chovatiya <savan.chovatiya@crestdatasys.com> Co-authored-by: Kyle Allan <KAllan357@gmail.com>
…6-update-custom-fields
7 participants
Description of change
Manual QA steps
Risks
Rollback steps