Skip to content
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

Raise exception if user includes data product not in registry in config #46

Merged
merged 5 commits into from
Aug 27, 2021

Conversation

DennisReddyhoff
Copy link
Collaborator

An exception should now be raised if user includes a data_product in write or run blocks that doesn't exist in the registry. Issue #34 @soniamitchell

@richardreeve
Copy link
Member

@DennisReddyhoff I haven't looked at the code, but it's okay not to raise an exception if a write doesn't exist - you may be writing a genuinely new thing. The corresponding working config should just give a version of 0.0.1 for PATCH, 0.1.0 for MINOR, etc.

@DennisReddyhoff
Copy link
Collaborator Author

@DennisReddyhoff I haven't looked at the code, but it's okay not to raise an exception if a write doesn't exist - you may be writing a genuinely new thing. The corresponding working config should just give a version of 0.0.1 for PATCH, 0.1.0 for MINOR, etc.

Oh yeah of course, in which case I'm not quite sure I understand the issue @soniamitchell raised

@richardreeve
Copy link
Member

It may be a typo. You should raise an exception if a version does not exist in a read block, and you should raise an exception if you specify an exact version and it does exist in a write block.

@soniamitchell
Copy link

I'm afraid I can't remember exactly what happened, I'll provide more detail next time.

I've just tested a data product that doesn't exist in a read: block and it generates a working config with version 0.0.0.

@richardreeve
Copy link
Member

I think that's covered in #44? This was about when fair pull / fair run should give errors I think...

@soniamitchell
Copy link

Should it not give an error if the data product doesn’t exist? Otherwise how will the user know?

@richardreeve
Copy link
Member

richardreeve commented Aug 26, 2021

Oh, sorry - yes. It was the comment about the write generating errors that I was concerned about that you mentioned in #34 - it's okay for a data product to not exist there, but you should raise an exception if you specify an exact version and it does exist in a write block. But maybe I misunderstood the issue you were raising.

@DennisReddyhoff
Copy link
Collaborator Author

Ok, for write an exception should now be raised if the version specified already exists in the registry and for read an exception should be raised if the data product doesn't exist in the registry

@soniamitchell
Copy link

on it

@soniamitchell
Copy link

perfect!

@soniamitchell
Copy link

Actually, one tiny comment. When reading a data product (that does exist) of a version that doesn't exist, the error message could list the version number like it does in the write block.

@DennisReddyhoff
Copy link
Collaborator Author

Ok cool, will fix that and merge

@sonarcloud
Copy link

sonarcloud bot commented Aug 27, 2021

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 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@DennisReddyhoff DennisReddyhoff merged commit 0369ed5 into dev Aug 27, 2021
@DennisReddyhoff DennisReddyhoff deleted the dev-checkexists branch August 27, 2021 12:47
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.

fair run doesn't check whether write block data products exist before allocating a version number
3 participants