33247 legal api dep upgrade#4387
Conversation
There was a problem hiding this comment.
NOTE: I am planning to remove all the model tests before merging as they should be secured in the common model tests.
I didn't want to do this before verifying that they were all passing here with minimal changes in case there were unknown differences etc
There was a problem hiding this comment.
NOTE: updates in here follow what was accepted in the common model - I think this is okay, but let me know if you disagree
There was a problem hiding this comment.
NOTE: these updates and other naics related test changes were already applied and approved in the common model as well and are just due to the test data populated being a little different in the migrations of the common model
There was a problem hiding this comment.
NOTE: logging is setup via our structured logging dependency - we removed this in our newer projects as well - I didn't dive too deep into whether or not to keep the older logging stuff around so let me know if you think I should bring it back
There was a problem hiding this comment.
NOTE: we use the version in the toml file instead now (same as newer projects)
There was a problem hiding this comment.
NOTE: similar changes made here that I made in the common model recently (full rollback of all transactions between each test) with one separate change needed for app_request
…esting, unit tests setup broken Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
kialj876
force-pushed
the
33247-legal-api-dep-upgrade
branch
from
37594a6 to
3322a8e
Compare
| return headers | ||
|
|
||
|
|
||
| @contextmanager | ||
| def jwt_request_context(app, |
There was a problem hiding this comment.
NOTE: there was a breaking change in the upgraded version of our flask-jwt-oidc package where we need to pass in custom request context data to some of the functions we use regularly in this API so this ensures that specific request context data is available for tests that skip over the client
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
|
|
Not sure why the CI isn't running the lint or unit tests here. It's supposed to run against 'feature*' branches. Below are my local runs: 
I've also tested the major functionality locally:
|
kialj876
requested review from
argush3,
loneil,
meawong,
thorwolpert and
vysakh-menon-aot
loneil
left a comment
loneil
left a comment
There was a problem hiding this comment.
Can't see any obvious blockers or fundamental issues. I've never had a chance to need to dig into migrations but assume that part of it is handled from the note here.
Probably comes down to regression testing then.
Issue #: /bcgov/entity#33247
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).