Added a ref Resolver #105

visch · 2022-01-08T16:50:00Z

Closes #104

…he old broken ref schemas, and added a tool for resolving the refs

visch · 2022-01-08T16:58:37Z

Worried about potential json schema failures as we haven't been able to fully test this with Data in production setups (Tested with my test instance and one live instances data, but history shows that's not enough)

visch · 2022-01-08T16:59:10Z

Check for any items in def's that don't' have a null option, add null as an option

visch

Need to fix those issues

visch · 2022-01-08T16:53:00Z

tap_clickup/schemas/folder.json

                    },
                    "color": {
                        "type": ["null", "string"]
                    },
                    "type": {
                        "type": ["null", "string"]
                    }
+            },
+        "feature": {


This doesn't look to be needed

tap_clickup/schemas/folder.json

visch · 2022-01-08T16:53:53Z

meltano.yml

@@ -13,6 +13,7 @@ plugins:
    - name: api_token
      kind: password
    select:
+    - '!shared_hierarchy.*'


tap_clickup/schemas/space.json

visch · 2022-01-08T16:56:49Z

tap_clickup/schemas/team.json

-                        "type": ["null", "string"]
-                    }
+                "id": {
+                    "type": "integer"


To be safe I think we should add null here, as these refs haven't been heavily tested by users.

visch · 2022-01-08T16:57:05Z

tap_clickup/schemas/team.json

-                        "user": {
-                            "$ref": "#/definitions/user"
+                        "id": {
+                            "type": "integer"


Add nulls (similar to above comment)

visch · 2022-01-08T16:57:11Z

tap_clickup/schemas/team.json

+                    "type": "object",
+                    "properties": {
+                        "id": {
+                            "type": "integer"


Add nulls (similar to above comment)

saeedzareian · 2022-02-18T06:17:03Z

Hi, thank you for the great work and support here.

I am using list (folder-list) and space heavily and this fix will help us a lot. Also, we found another issue that I will share shortly with you after verifying my solution. Could you please move forward with this pull request earlier?

visch · 2022-02-18T13:47:03Z

Hi, thank you for the great work and support here.

I am using list (folder-list) and space heavily and this fix will help us a lot. Also, we found another issue that I will share shortly with you after verifying my solution. Could you please move forward with this pull request earlier?

Yeah I want to merge this, I just need to test that it works with target-postgres, and target-stitch. Issue is that I don't have functional testing setup to automate this for me (and I'd probably want to smoke test myself locally anyway)

Next steps are

Verify data before and after this MR are the same in target-postgres (ie select * from old_tap_clickup except select * from new_tap_clickup and visaversa should return 0 records) , looks like CHECKSUM TABLE old_tap_clickup , new_tap_clickup ; would work too, if there are differences we should look at them and see if they make sense or not.
Run tap-postgres target-stitch to be sure data gets through so I don't break anyone later
Run tap-postgres target-jsonl to be sure schema checks pass

I won't have time to do this for a bit. I''ll try to, or if someone could do these tests for me or a portion of them it'd be mightly helpful!

tap_clickup/schemas/space.json

visch · 2022-02-18T13:52:34Z

tap_clickup/schemas/team.json

                        },
-                        "invited_by": {
-                            "$ref": "#/definitions/user"


Would be nice to keep this instead of duplicating what a user is

noted in above comment

visch · 2022-02-18T13:52:40Z

tap_clickup/schemas/team.json

-            "type": "array",
-            "items": [
-                {
-                    "type": "object",


Would be nice to keep this instead of duplicating what a user is

visch · 2022-02-18T13:53:49Z

Hi, thank you for the great work and support here.

I am using list (folder-list) and space heavily and this fix will help us a lot. Also, we found another issue that I will share shortly with you after verifying my solution. Could you please move forward with this pull request earlier?

There's also some duplicate in the PR, I'd like to remove some of them and replace them with Refs, if someone could to that it would be very helpful as well (I hope it can work without changing the ref resolver we're using, if not we don't need to dive too hard we can just leave the duplication)

saeedzareian · 2022-02-20T14:31:51Z

I won't have time to do this for a bit. I''ll try to, or if someone could do these tests for me or a portion of them it'd be mightly helpful!

I will ask my friends if they have capacity to help, their sprint starts the next Monday (No tomorrow).

visch · 2022-03-03T00:20:59Z

Data looks good from a run on my end

Check for any items in def's that don't' have a null option, add null as an option

After review there were only a handful of properties without nulls, this shouldn't' hold up the merge as this PR puts us in a better place.

visch · 2022-03-03T00:43:20Z

I won't have time to do this for a bit. I''ll try to, or if someone could do these tests for me or a portion of them it'd be mightly helpful!

I will ask my friends if they have capacity to help, their sprint starts the next Monday (No tomorrow).

We're all merged in, pull the latest from pypi and you should be good to go!

Refs were not working in Json Schema, added a ref resolver, updated t…

c5cb131

…he old broken ref schemas, and added a tool for resolving the refs

visch commented Jan 8, 2022

View reviewed changes

visch added 2 commits January 8, 2022 12:08

Added nulls to be as safe as possible in the definitions

f08b11b

noticed a typo in json

e606837

visch commented Feb 18, 2022

View reviewed changes

tap_clickup/schemas/space.json Show resolved Hide resolved

visch commented Feb 18, 2022

View reviewed changes

visch merged commit 6452c25 into main Mar 3, 2022

visch deleted the snowflake-issues branch April 25, 2022 12:55

visch restored the snowflake-issues branch April 25, 2022 12:55

visch deleted the snowflake-issues branch April 25, 2022 12:56

visch restored the snowflake-issues branch April 25, 2022 12:56

visch deleted the snowflake-issues branch April 25, 2022 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a ref Resolver #105

Added a ref Resolver #105

visch commented Jan 8, 2022

visch commented Jan 8, 2022

visch commented Jan 8, 2022 •

edited

Loading

visch left a comment

visch Jan 8, 2022

visch Jan 8, 2022

visch Jan 8, 2022

visch Jan 8, 2022

visch Jan 8, 2022

saeedzareian commented Feb 18, 2022 •

edited

Loading

visch commented Feb 18, 2022

visch Feb 18, 2022

visch Mar 3, 2022

visch Feb 18, 2022

visch commented Feb 18, 2022

saeedzareian commented Feb 20, 2022

visch commented Mar 3, 2022

visch commented Mar 3, 2022

Added a ref Resolver #105

Added a ref Resolver #105

Conversation

visch commented Jan 8, 2022

visch commented Jan 8, 2022

visch commented Jan 8, 2022 • edited Loading

visch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saeedzareian commented Feb 18, 2022 • edited Loading

visch commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

visch commented Feb 18, 2022

saeedzareian commented Feb 20, 2022

visch commented Mar 3, 2022

visch commented Mar 3, 2022

visch commented Jan 8, 2022 •

edited

Loading

saeedzareian commented Feb 18, 2022 •

edited

Loading