Skip to content

TDL-21114 add tags property for submitted_landings stream#75

Merged
kethan1122 merged 8 commits intomasterfrom
TDL-21114-add-tags-property
May 31, 2023
Merged

TDL-21114 add tags property for submitted_landings stream#75
kethan1122 merged 8 commits intomasterfrom
TDL-21114-add-tags-property

Conversation

@kethan1122
Copy link
Contributor

@kethan1122 kethan1122 commented May 25, 2023

Description of change

Duplicates #68

Manual QA steps

  • Created tags for responses available for different surveys on test account and ran sync to see field being extracted.

Risks

  • Low

Rollback steps

  • revert this branch

Comment on lines +37 to +45
"tags": {
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"tags": {
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
"tags": {
"type": [
"null",
"array"
],
"items": {
"type": [
"null",
"string"
}
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

@kethan1122 kethan1122 requested a review from RushiT0122 May 25, 2023 08:21
Add additional data and nested fields to top level
"""
record.update({
"tags": record["tags"] if "tags" in record else [],
Copy link
Contributor

Choose a reason for hiding this comment

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

If "tags" doesn't exist on the record we should return None, not an empty list. Changing this to record.get("tags") would work.

Copy link
Contributor Author

@kethan1122 kethan1122 May 25, 2023

Choose a reason for hiding this comment

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

ok.. in that case is it necessary to have null as data type here ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we should leave the null option in the schema

Comment on lines +37 to +48
"tags": {
"type": [
"null",
"array"
],
"items": {
"type": [
"null",
"string"
]
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't realise it earlier but to keep pattern uniform, can we write schema on the same line?

Otherwise it looks good.

"tags": {
"type": ["null", "array"],
"items": {
"type": ["string"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you misunderstood my last comment, the null should remain in the schema. That will allow null values to persisted if there are any. As it is written here if a null was returned by the API the transformer would throw an error.

@kethan1122 kethan1122 merged commit ad804ac into master May 31, 2023
@kethan1122 kethan1122 deleted the TDL-21114-add-tags-property branch May 31, 2023 15:20
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.

3 participants