-
Notifications
You must be signed in to change notification settings - Fork 52
Adding UI validation functions for operatorhub.io #36
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
Conversation
SamiSousa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The descriptions for each missing field will be really helpful, great work!
| return True | ||
|
|
||
| def is_version(field): | ||
| pattern1 = re.compile(r'v(\d+\.)(\d+\.)(\d)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe version also supports things like -alpha that can be added to the end of a semver.
Can these patterns be consolidated? Eg: r'v?(\d+\.)(\d+\.)(\d)(-[a-z]+)?'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. @jeff-phillips-18 can you confirm that this is allowed for the UI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it may be better to check with this package: https://pypi.org/project/semver/
We should use a regex that matches what quay.io is using to be complete (the above isn't that :/).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that would work for the UI. We only really use it to sort the different versions, for version selection dropdown and for determining the latest version.
720e422 to
7dfb215
Compare
operatorcourier/api.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
| def build_and_verify(source_dir=None, yamls=None): | ||
| def build_and_verify(source_dir=None, yamls=None, ui_validate_io=False, ui_validate_ocp=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be ui_validate_ocp or ui_validate_okd ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be. I'll wait to see what others say before changing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could also simply call it ui_validate_openshift
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 can this be ui_validate_openshift? I don't think this is ocp specific.
3396409 to
b78a165
Compare
|
Removed the WIP tag. @awgreene @SamiSousa @dmesser @kevinrizza @aravindhp @jeff-phillips-18 @anik120 @JEREMYLINLIN Please review |
dmesser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
operatorcourier/api.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
| def build_and_verify(source_dir=None, yamls=None): | ||
| def build_and_verify(source_dir=None, yamls=None, ui_validate_io=False, ui_validate_ocp=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 can this be ui_validate_openshift? I don't think this is ocp specific.
|
/lgtm |
I'm adding the WIP tag because I am adding some tests for the UI, but @kevinrizza you're welcome to start reviewing whenever you have time.
/cc @kevinrizza @jeff-phillips-18 @awgreene @dmesser