Skip to content

Add csi-addons Phase 0 prerequisites: replication status read, idempo… - #1372

Open
geoffrey1330 wants to merge 10 commits into
mainfrom
integrate_csi_addons_p0
Open

geoffrey1330 wants to merge 10 commits into
mainfrom
integrate_csi_addons_p0

Conversation

@geoffrey1330

Copy link
Copy Markdown
Collaborator

…tent attach/detach, rpo_target_seconds

@mxsrc mxsrc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some notes about the types used in the API. Please try to use type annotations on all functions you introduce.

Comment thread simplyblock_web/api/v2/_dtos.py Outdated
Comment thread simplyblock_web/api/v2/_dtos.py Outdated
Comment on lines +700 to +702
#: The declared RPO objective (P0-4); null when the operator declared
#: none, in which case the derived lag budget applies.
rpo_target_seconds: int | None = None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this mean the value can only be in the [0,4] interval? If that is the case, please annotate that constraint.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, the P0-4 is the phase( phase 0 item 4). this how i track the task to implement. it makes it easier for agent to keep track of what is implemented and what is not yet.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see. That makes perfect sense but it shouldn't end up in the code. Without the planning document you're iterating on it's impossible to understand what's meant.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Alright. I will remove them before the merge

Comment thread simplyblock_web/api/v2/cluster/storage_pool/volume/replication.py Outdated

@mxsrc mxsrc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks!

…emote endpoint and planned-gate force-escalation fix
raise HTTPException(500, str(result[1]))
if result["demoted"]:
return Response(status_code=204)
return JSONResponse(status_code=202, content=result)
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.

3 participants