Skip to content

Conversation

@bruvio
Copy link
Collaborator

@bruvio bruvio commented Jan 12, 2022

relates to #13

  1. PR improves test coverage
  2. tries to handle exception in part of the code when the API assumes variables are in the correct state.
  3. removed use of reserved work 'type' changed to 'issue_type'

bruvio and others added 6 commits January 11, 2022 16:55
handling get entry exception
increase test coverage adding more tests

removed use of reserved work 'type' changed to 'issue_type'

handling of exception when API assumes that variables are in the correct state when accessing indexes
@bruvio bruvio added the enhancement New feature or request label Jan 12, 2022
@bruvio bruvio requested a review from RyanJField January 12, 2022 15:51
@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #30 (126960d) into dev (ea61065) will increase coverage by 1.82%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #30      +/-   ##
==========================================
+ Coverage   84.89%   86.71%   +1.82%     
==========================================
  Files           5        5              
  Lines         503      512       +9     
==========================================
+ Hits          427      444      +17     
+ Misses         76       68       -8     
Impacted Files Coverage Δ
fairdatapipeline/__init__.py 100.00% <ø> (ø)
fairdatapipeline/fdp_utils.py 84.23% <75.00%> (-0.30%) ⬇️
fairdatapipeline/pipeline.py 92.02% <75.00%> (+0.30%) ⬆️
fairdatapipeline/raise_issue.py 80.48% <100.00%> (+10.97%) ⬆️

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 ea61065...126960d. Read the comment docs.

bruvio added 2 commits January 13, 2022 07:26
…ct_id function

catching config and script exceptions in initialise pipeline
@bruvio bruvio requested a review from kzscisoft January 13, 2022 08:55
"""
return list(filter(None, urlsplit(url).path.split("/")))[-1]
try:
return list(filter(None, urlsplit(url).path.split("/")))[-1]
Copy link
Collaborator

@kzscisoft kzscisoft Jan 13, 2022

Choose a reason for hiding this comment

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

list(filter( doesn't do anything in this context and is actually confusing, the purpose of this is to simply get the last element of a list, and if the list is empty raise an exception, as urlsplit(url).path.split("/") already returns a list that is all that is needed. I do not know why filter is being used at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, using your idea.
split_url_path = urlsplit(url).path.split("/")
if not split_url_path:
raise IndexError(f"Unable to extract ID from registry URL: {url}")
return [s for s in split_url_path if s.strip() != ''][-1]

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bruvio bruvio requested a review from kzscisoft January 13, 2022 09:53
@kzscisoft kzscisoft merged commit f919b2c into dev Jan 13, 2022
@kzscisoft kzscisoft deleted the feature/error-handling branch January 13, 2022 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants