Skip to content

[FIX] Removed XNAT integration (was unused)#288

Merged
DESm1th merged 3 commits intoTIGRLab:masterfrom
DESm1th:link_fix
Oct 1, 2020
Merged

[FIX] Removed XNAT integration (was unused)#288
DESm1th merged 3 commits intoTIGRLab:masterfrom
DESm1th:link_fix

Conversation

@DESm1th
Copy link
Copy Markdown
Contributor

@DESm1th DESm1th commented Sep 28, 2020

We havent used the XNAT integration for linked IDs since the dashboard was made, and now that we're moving studies to the KCNI server it is causing crashes so I've removed it.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 28, 2020

Codecov Report

Merging #288 into master will increase coverage by 0.14%.
The diff coverage is 43.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
+ Coverage   31.88%   32.02%   +0.14%     
==========================================
  Files          57       57              
  Lines        8679     8647      -32     
==========================================
+ Hits         2767     2769       +2     
+ Misses       5912     5878      -34     
Impacted Files Coverage Δ
bin/dm_link_shared_ids.py 38.46% <31.81%> (+4.45%) ⬆️
tests/test_dm_link_shared_ids.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93c2d7a...2e37c12. Read the comment docs.

@DESm1th DESm1th requested review from jerdra and josephmje September 29, 2020 15:47
josephmje
josephmje previously approved these changes Sep 30, 2020
Copy link
Copy Markdown
Contributor

@josephmje josephmje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @DESm1th! Looking good to me. Only question I had is whether there's a reason for switching the logging from StreamHandler to basic config? I don't have any preference on this. I've just been following what was done in the past.

Comment thread bin/dm_link_shared_ids.py Outdated
try:
project_records = parse_records(response, current_study, id_map)
except ValueError as e:
logger.error("Couldnt parse redcap records for server response {}. "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.error("Couldnt parse redcap records for server response {}. "
logger.error("Couldn't parse redcap records for server response {}. "

@DESm1th
Copy link
Copy Markdown
Contributor Author

DESm1th commented Sep 30, 2020

It's just less verbose. basicConfig just adds a streamhandler.

@jerdra
Copy link
Copy Markdown
Contributor

jerdra commented Oct 1, 2020

looks much cleaner now! thanks for the refactor Dawn 🥇

@DESm1th DESm1th merged commit 0b8840d into TIGRLab:master Oct 1, 2020
@DESm1th DESm1th deleted the link_fix branch October 1, 2020 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants