-
Notifications
You must be signed in to change notification settings - Fork 4
Update pipeline import to create a new version instead of patching #291
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
30d0476 to
4f8eb80
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
| version_body = version_response.json() | ||
| latest_version = version_body["data"][0] | ||
| version_id = latest_version["version_id"] | ||
| is_draft = latest_version.get("is_draft", 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.
We are assuming the response is a valid response without checking. If we get another status code like, say, 401 or 403 these lines may raise an error.
Instead let's do the following:
import httpx
try:
version_response = await sef._api.get(...)
version_response.raise_for_status()
except httpx.HTTPStatusError as e:
if e.response.status_code != HTTPStatus.NOT_FOUND:
raise
# the pipeline does not exist, let's create it.
return await self._api.create_pipeline(...)
# pass the response
version_body = ...
if is_draft:
# do draft things
return
# do non-draft thingsThere 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.
Hi @abrahamy Updated this accordingly.
Related Issues
Proposed Changes?
Previously, in #283, we updated the pipeline import to fetch the latest pipeline version and patch that.
Instead, we want the following behavior:
How did you test it?
Changes in unit and integration tests.
And tried out via the import script.
Checklist