-
Notifications
You must be signed in to change notification settings - Fork 62
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
cli: dont check dirname presence in action_build #3106
Conversation
cli/copr_cli/main.py
Outdated
# /build/check-before-build endpoint checks for presence of this | ||
# dirname even before creating it in this check. Passing only projectname | ||
# should be sufficient for the comment above. | ||
project_dirname=projectname, |
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.
Have you tried to call this with project_dirname=None
?
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 and it failed with Error: The browser (or proxy) sent a request that this server could not understand
. If project_dirname
is not passed to cli and only projectname is passed in either ownername/projectname
or only projectname
format then the project_dirname is not None
but projectname
.
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.
This is a frontend API bug, isn't it?
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.
Note this:
copr/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py
Lines 230 to 232 in 3b68ad1
# Raises an exception if CoprDir doesn't exist | |
if data["project_dirname"]: | |
CoprDirsLogic.get_by_copr(copr, data["project_dirname"]) |
Otherwise looks good to me! |
caf7187
to
715f7fa
Compare
python/copr/v3/proxies/build.py
Outdated
@@ -162,6 +162,9 @@ def check_before_build(self, ownername, projectname, | |||
:param buildopts: http://python-copr.readthedocs.io/en/latest/client_v3/build_options.html | |||
:return: Munch | |||
""" | |||
if project_dirname is None: | |||
project_dirname = projectname |
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 don't want to nitpick, but what happens if project_dirname=None
goes down to the Frontend API code? It seems we are hiding a Frontend API bug here ...
Copr build <projectname:dirname> should according to documentation create the dirname if does not exists, but the early check check_before_build in action_build actually runs /build/check-before-build endpoint even before the <projectname:dirname> has a chance to be created, thus complains about non-existent dirname. Passing only projectname to the check should be enough for the check as is since the /build/check-before-build is called once more after dirname creation. Fixes fedora-copr#2786
715f7fa
to
74fb55e
Compare
@@ -228,7 +228,7 @@ def check_before_build(): | |||
copr = get_copr() | |||
|
|||
# Raises an exception if CoprDir doesn't exist | |||
if data["project_dirname"]: | |||
if data.get("project_dirname"): |
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.
👍
Copr build projectname:dirname should according to documentation create the dirname if does not exists, but the early check check_before_build in action_build actually runs /build/check-before-build endpoint even before the projectname:dirname has a chance to be created, thus complains about non-existent dirname.
Passing only projectname to the check should be enough for the check as is since the /build/check-before-build is called once more after dirname creation.
Fixes #2786