Skip to content

enhancements on scicrunch classifiers (backend) #2065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Dec 23, 2020

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Dec 18, 2020

What do these changes do?

Some enhancements and increases test coverage on scicrunch classifiers

Fixes error responses

  • Possible outcomes from POST /groups/sparc/classifiers/scicrunch-resources/{rrid}
  • IMPORTANT: rrid should NOT have prefix RRID:
responses:
        "200":
          description: Got information of a valid RRID
        "400":
          description: Invalid RRID
        "503":
          description: scircrunch.org service is not reachable

non "SCR_" RRIDs

  • scicrunch.org API only works with RRIDs defined in scicrunch registry tools (prefixed with SCR_)
  • The rest are all treated as invalid identifiers even if they can be searched via their website
  • See trials and classification done in services/web/server/tests/unit/isolated/test_scicrunch_service_api.py

Related issue/s

How to test

Get a token in scicrunch.org for API

export SCICRUNCH_API_KEY=some-secret-token
make devenv
cd services/web/server
make install-dev
make test-dev-unit

Checklist

  • @GitHK reviews from New classifiers with scicrunch RRIDs (⚠️ devops) #2045
  • Fix: if auth error (e.g. invalid token) now it is returning notfound!
  • Check queries with non SCR_ RRIDs
  • Did you change any service's API? Then make sure to bundle document and upgrade version (make openapi-specs, git commit ... and then make version-*)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes
  • New module? Add your github username to .github/CODEOWNERS

@pcrespov pcrespov self-assigned this Dec 18, 2020
@pcrespov pcrespov added the a:sidecar issue related with the sidecar worker service label Dec 18, 2020
@codecov
Copy link

codecov bot commented Dec 18, 2020

Codecov Report

Merging #2065 (ffa547f) into master (16c4a86) will increase coverage by 0.1%.
The diff coverage is 38.7%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2065     +/-   ##
========================================
+ Coverage    72.7%   72.9%   +0.1%     
========================================
  Files         421     421             
  Lines       15463   15487     +24     
  Branches     1563    1568      +5     
========================================
+ Hits        11255   11295     +40     
+ Misses       3805    3785     -20     
- Partials      403     407      +4     
Flag Coverage Δ
integrationtests 63.5% <24.8%> (-0.2%) ⬇️
unittests 67.4% <38.7%> (+0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...src/simcore_service_webserver/scicrunch/_config.py 100.0% <ø> (ø)
...r/src/simcore_service_webserver/groups_handlers.py 64.1% <6.0%> (-6.6%) ⬇️
...rc/simcore_service_webserver/groups_classifiers.py 50.0% <50.0%> (ø)
...core_service_webserver/scicrunch/service_client.py 60.2% <50.8%> (+22.1%) ⬆️
...re_service_webserver/scicrunch/scicrunch_models.py 87.6% <86.6%> (+11.8%) ⬆️
...ore_service_webserver/scicrunch/submodule_setup.py 100.0% <100.0%> (+15.3%) ⬆️
...simcore_service_webserver/projects/projects_api.py 84.6% <0.0%> (-0.5%) ⬇️
...ce_webserver/resource_manager/garbage_collector.py 77.5% <0.0%> (ø)
... and 2 more

@pcrespov pcrespov force-pushed the enh/scicrunch-classifiers branch from 8885328 to 70971ec Compare December 23, 2020 00:47
@pcrespov pcrespov force-pushed the enh/scicrunch-classifiers branch from 70971ec to 0d685d4 Compare December 23, 2020 00:57
@pcrespov pcrespov added a:webserver issue related to the webserver service bug buggy, it does not work as expected t:enhancement Improvement or request on an existing feature and removed a:sidecar issue related with the sidecar worker service bug buggy, it does not work as expected labels Dec 23, 2020
@pcrespov pcrespov marked this pull request as ready for review December 23, 2020 08:37
@pcrespov pcrespov requested a review from GitHK December 23, 2020 08:37
@pcrespov pcrespov changed the title enhancements on scicrunch classifiers enhancements on scicrunch classifiers (backend) Dec 23, 2020
@odeimaiz odeimaiz added this to the Christmas'20 milestone Dec 23, 2020
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

👏 🎩

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

💪

@pcrespov pcrespov merged commit 691f4c8 into ITISFoundation:master Dec 23, 2020
Copy link
Member

@mguidon mguidon left a comment

Choose a reason for hiding this comment

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

Go ahead.

@pcrespov pcrespov deleted the enh/scicrunch-classifiers branch December 23, 2020 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service t:enhancement Improvement or request on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants