-
Notifications
You must be signed in to change notification settings - Fork 22
DBCFeeder Authorization Support #74
DBCFeeder Authorization Support #74
Conversation
d8aafcd
to
8c9d418
Compare
@lukasmittag @SebastianSchildt - for information - this adapts dbcfeeder to authorization. The refactoring here could possibly also be reused by other python-based feeders like ddsfeeder, i.e. that they just re-use the wrappers. Not ready for final review yet |
4fc26bd
to
2c64f90
Compare
9891c97
to
aeb7eb5
Compare
Also major refactoring to move client-specific code out of main file.
aeb7eb5
to
81ec953
Compare
Old mapping format no longer supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is here different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Github is not that good at showing subtle differences. If you use for example gitk you will see that a newline has been added.
Feeder refactored and new mapping format based on VSS introduced, see [documentation](mapping.md).
-Old mapping format no longer supported.
\ No newline at end of file
+Old mapping format no longer supported.
|
||
# TODO add data for root cert if using TLS and if given | ||
|
||
self._kuksa = KuksaClientThread(self._client_config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to us threaded API here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is for KUKSA.val server (i.e. websocket) and has not been changed in this PR - the server-specific code has just been put in separate file to make dbcfeeder.py
unaware of what APIs that actually are used. What other options do we have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, I missed that. fought it was in the databrokerclientwrapper. Then its fine
@@ -0,0 +1,16 @@ | |||
name: pre-commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SebastianSchildt is this okay from an open source legal perspective? Tp run automatically run a code formatter before every commit? We had the dicussion regarding auto generation of licenses too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually does not fix anything when run as part of CI. See the dummy PR in erikbosch#2 where I have added a blank in a README. The check will fail, it will not fix it.
This can be compared to if you run it locally. There it will fix the file, but it will not do a git add
and it will not do a git commit --amend
. So also when running locally you must "accept" the changes (by git add) and then try git commit
again.
erik@debian3:~/kuksa.val.feeders$ pre-commit run --file README.md
Trim Trailing Whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook
Fixing README.md
Fix End of Files.........................................................Passed
Check Yaml...........................................(no files to check)Skipped
Check for added large files..............................................Passed
flake8...............................................(no files to check)Skipped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, it read like it would fix it. But then also fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me as well.
This PR does:
It uses the kuksa-client pre-release https://pypi.org/project/kuksa-client/0.4.0a1/. Later as part of release process we should make sure that it uses an official version instead.