RM-42 job config schema too complex

[Brunope]

No description provided.

[Brunope]

Any advice on this @ryantimjohn? I'm not too happy with it so far. All the extra params needing to be passed around plus the type-checking info makes it more convoluted than I'd like. Also it's still "too complex". There's some logic around the arg_name that is hard to extract.

[Brunope]

This branch isn't unit tested

[ryantimjohn]

Any advice on this @ryantimjohn? I'm not too happy with it so far. All the extra params needing to be passed around plus the type-checking info makes it more convoluted than I'd like. Also it's still "too complex". There's some logic around the arg_name that is hard to extract.

I was thinking about this with my own over complex function and read somewhere online "if you're passing around too many parameters you might have an object sitting in there." I think I'm going to work a parameters object into mine, don't know if that's helpful here.

[ryantimjohn]

As for extracting the arg_name logic, don't see it right away but happy to jump on to paired sometime, will Slack you!

[Brunope]

Odd, getting a bunch of these errors, I don't think it's related to my changes but it's possible

FAILED test_records_unload_datetime.py::RecordsUnloadDatetimeIntegrationTest::test_unload_datetime - sqlalchemy.exc.ProgrammingError: (psycopg2.errors.InsufficientPrivilege) permission denied for relation svv_table_info

[SQL: 
                        SELECT table_id
                        FROM
                            SVV_TABLE_INFO
                        WHERE 1
                            AND schema = 'public'
                            AND "table" = '*****_24498_1680105943';
                        ]
(Background on this error at: https://sqlalche.me/e/14/f405)

[ryantimjohn]

Odd, getting a bunch of these errors, I don't think it's related to my changes but it's possible

FAILED test_records_unload_datetime.py::RecordsUnloadDatetimeIntegrationTest::test_unload_datetime - sqlalchemy.exc.ProgrammingError: (psycopg2.errors.InsufficientPrivilege) permission denied for relation svv_table_info

[SQL: 
                        SELECT table_id
                        FROM
                            SVV_TABLE_INFO
                        WHERE 1
                            AND schema = 'public'
                            AND "table" = '*****_24498_1680105943';
                        ]
(Background on this error at: https://sqlalche.me/e/14/f405)

Very bizarre, wonder if one of the tokens died in the background or something but... that seems extremely unlikely.

[Brunope]

Really not a huge fan of setting self vars outside of __init__, but it does make the rest of the code easier. To move it to __init__ would require making new overloaded init functions that accept more params, which I'm not sure I want to do

[ryantimjohn]

that makes sense. Does adding a setter method to the class make sense? Then, at least, when assessing the class's code, you'd know what potential attributes the class had?

[Brunope]

Could you explain a bit more? I'm not really sure what you mean

[ryantimjohn]

Sorry, please ignore last comment, wrote very quickly.

Basically, I wonder if it just makes sense to break out the piece where you assign the required_keys attribute into a separate set_required_keys method and then just call that method in the configure_from_properties method. Not sure it's worth the trouble.

[Brunope]

Ah, sure, is the goal simply to make it more explicit?

[ryantimjohn]

yep!

[ryantimjohn]

Changes look good! A lot easier to read now.