Skip to content

Conversation

@ajreid21
Copy link
Contributor

@ajreid21 ajreid21 commented Nov 12, 2025

Adding to the spec a way to pass the client credentials specific to the Scan Plan results. Both the planTableScan and fetchPlanningResult return a CompletedPlanningResult when scan plan is completed.

ML discussion: https://lists.apache.org/thread/ko9kp0gvzhx85n7cvoxqnpw4vwnhmdg6

@singhpk234 singhpk234 self-requested a review November 12, 2025 03:45
status:
$ref: '#/components/schemas/PlanStatus'
const: "completed"
storage-credentials:
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should have description here stating these creds are subscoped to scan or something, thoughts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure the spec should dictate or be opinionated that the creds are specifically subscoped, just that the client should be able to use these creds to access files returned in the scan result, but if others feel strongly, I'm not totally opposed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we shouldn't have wording in the spec saying that the creds are subscoped. It is up to the server to subscope those or not

Copy link
Contributor

@singhpk234 singhpk234 Nov 12, 2025

Choose a reason for hiding this comment

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

my understanding if the creds are present in the scan response we should not use the creds that were vended in loadTable ? hence some literature around of these cred should be the one used when server returns it irrespective of what was vended from the loadTable ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a description, let me know if that works or it needs re-wording

"""

status: Literal['completed'] = Field(..., const=True)
storage_credentials: Optional[List[StorageCredential]] = Field(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put these as optional, anyone have thoughts or opinions if these should be required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants