Feature.extendprecommit #375
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| require_serial: true | ||
| verbose: true | ||
| args: | ||
| - --no-warn-incomplete-stub |
There was a problem hiding this comment.
Were these required for cnaas-nms or like a basic config copied from somewhere?
There was a problem hiding this comment.
The args have been checked and are needed for cnaas-nms. Might be good to keep an eye on that. It could be a good idea to check if the issues we ignore now could be good code improvements, but for now I thought this code improvement was getting really big and it was better to improve it incremental. The other settings I took from Workflow Orchestrator.
| @@ -566,7 +568,7 @@ def post(self, device_id: int): | |||
|
|
|||
| if mlag_peer_id: | |||
| try: | |||
| mlag_peer_dev: Device = cnaas_nms.devicehandler.init_device.pre_init_checks(session, mlag_peer_id) | |||
| mlag_peer_dev = cnaas_nms.devicehandler.init_device.pre_init_checks(session, mlag_peer_id) | |||
There was a problem hiding this comment.
We already defined the name with type. We don't want to redefine the variable. We are also not allowed to redefine it the same type.
| @@ -31,14 +31,14 @@ def update_interfacedb_worker( | |||
| replace: bool, | |||
| delete_all: bool, | |||
| mlag_peer_hostname: Optional[str] = None, | |||
| linknets: Optional[List[dict]] = None, | |||
| linknets: List[dict] = [], | |||
There was a problem hiding this comment.
same here with list as default argument
There was a problem hiding this comment.
We could make it None, but it does mean you need a lot of extra checking on the type. I thought the empty list had the same result, without having 2 types for one variable. If you prefer the None, I could add the checks and work like that.
| with sqla_session() as session: | ||
| dev: Device = session.query(Device).filter(Device.hostname == hostname).one() | ||
| with sqla_session() as session: # type: ignore | ||
| dev = session.query(Device).filter(Device.hostname == hostname).one() |
There was a problem hiding this comment.
We defined the variable type earlier in the same function. The best solution would maybe be to split the function up in smaller functions as we now do a lot in one function. But for now it works if we don't redefine the type.
|
Ectended the pre-commit and solved the issues that mypy marked