Skip to content
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

fix: provide deprecation msg to max_vfolder_size graphene field #1644

Merged
merged 6 commits into from
Oct 23, 2023

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Oct 23, 2023

follow up
#1397
#1643

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • API server-client counterparts (e.g., manager API -> client SDK)

@fregataa fregataa added this to the 23.09 milestone Oct 23, 2023
@fregataa fregataa self-assigned this Oct 23, 2023
@github-actions github-actions bot added comp:manager Related to Manager component size:S 10~30 LoC labels Oct 23, 2023
@yomybaby
Copy link
Member

Currently, max_vfolder_count and max_quota_scope_size in [Create|Modify]ProjectResourcePolicyInput and [Create|Modify]UserResourcePolicyInput are required fields. However, it would be better to change them to optional fields unless there is a specific reason for them to be required. @fregataa
The required field in the input type can break backward compatibility.

@github-actions github-actions bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Oct 23, 2023
@fregataa
Copy link
Member Author

Currently, max_vfolder_count and max_quota_scope_size in [Create|Modify]ProjectResourcePolicyInput and [Create|Modify]UserResourcePolicyInput are required fields. However, it would be better to change them to optional fields unless there is a specific reason for them to be required. @fregataa The required field in the input type can break backward compatibility.

I didn't check those parts. I updated the code, thank you!

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

As #1643 is merged, please update the merge conflict. (max_vfolder_count's GQL type is changed from BigInt to grpahene.Int)

@fregataa
Copy link
Member Author

As #1643 is merged, please update the merge conflict. (max_vfolder_count's GQL type is changed from BigInt to grpahene.Int)

Thank you, I resolved the part and pushed new commits.

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

Multi-line news fragment will break the formatting when auto-generating changelogs. Please put them in a single line, and contract as one sentence.

Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM!

GraphQL schema has changed as expected.

type ProjectResourcePolicy {
  id: ID!
  name: String!
  created_at: DateTime!
  max_vfolder_count: BigInt
  max_vfolder_size: BigInt @deprecated(reason: "Deprecated since 23.09.1")
  max_quota_scope_size: BigInt
}

@github-actions github-actions bot added size:S 10~30 LoC and removed size:M 30~100 LoC labels Oct 23, 2023
@fregataa fregataa added this pull request to the merge queue Oct 23, 2023
Merged via the queue into main with commit 10662e8 Oct 23, 2023
21 checks passed
@fregataa fregataa deleted the fix/max-vfolder-size-to-deprecate branch October 23, 2023 12:55
fregataa added a commit that referenced this pull request Oct 23, 2023
)

Backported-from: main
Backported-to: 23.09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component size:S 10~30 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants