ci: Configure numtracker as part of compose - #1641
Conversation
525f2d4 to
912db5b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## update-keycloak #1641 +/- ##
================================================
Coverage 96.34% 96.34%
================================================
Files 46 46
Lines 3666 3666
================================================
Hits 3532 3532
Misses 134 134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Alexj9837
left a comment
Alexj9837
left a comment
There was a problem hiding this comment.
Looks good, as some fall backs and takes whats in the just file and adds it into configure_numtracker
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
Comments are mostly minor but can this PR be merged after the other convert-to-python one so that they stay separate in the history
| "--detector" '{instrument}-{scan_number}-{detector}' | ||
| "--number" "43") | ||
|
|
||
| services: compose configure-adsim |
There was a problem hiding this comment.
This is going to play havoc with muscle memory :/. Are there any docs that mention just services that will need updating?
| config: {directory: "/tmp/", | ||
| scan: "{instrument}-{scan_number}", | ||
| detector: "{instrument}-{scan_number}-{detector}", | ||
| scanNumber: 43}) { |
There was a problem hiding this comment.
| config: {directory: "/tmp/", | |
| scan: "{instrument}-{scan_number}", | |
| detector: "{instrument}-{scan_number}-{detector}", | |
| scanNumber: 43}) { | |
| config: {directory: "/tmp/", | |
| scan: "{instrument}-{scan_number}", | |
| detector: "{instrument}-{scan_number}-{detector}", | |
| scanNumber: 43}) { |
|
|
||
|
|
||
| def configure_numtracker(): | ||
| def _(): |
There was a problem hiding this comment.
Why wrap this in a function? Isn't it only being used once?
| response.raise_for_status() | ||
| return response.json().get("access_token") | ||
|
|
||
| nt_url = os.environ.get("NT_URL", "http://localhost:8406/graphql") |
There was a problem hiding this comment.
Could this go with the other constants at the top of the module?
|
|
||
| nt_url = os.environ.get("NT_URL", "http://localhost:8406/graphql") | ||
| response = requests.post( | ||
| str(nt_url), |
There was a problem hiding this comment.
Isn't the url already a string?
Just seen there is a 'enqueue stack' option which might do the same thing - but either way, it would be good to see this as a separate change when everything else is merged
3 participants
Configure numtracker as part of compose services