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

Force register a new schema #1877

Merged
merged 16 commits into from
Oct 17, 2023
Merged

Conversation

aindriu-aiven
Copy link
Contributor

@aindriu-aiven aindriu-aiven commented Oct 13, 2023

Force register a new schema also auto select the default partition and replication numbers on a new topic request.

Linked issue

Resolves: #1882

What kind of change does this PR introduce?

  • Bug fix
  • New feature
  • Refactor
  • Docs update
  • CI update

What is the current behavior?

Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).

What is the new behavior?

Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).

Other information

Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.

Requirements (all must be checked before review)

  • The pull request title follows our guidelines
  • Tests for the changes have been added (if relevant)
  • The latest changes from the main branch have been pulled
  • pnpm lint has been run successfully

@programmiri programmiri linked an issue Oct 13, 2023 that may be closed by this pull request
@programmiri programmiri force-pushed the issue-force-register-on-new-schema branch from 431c350 to a931abb Compare October 16, 2023 04:19
if($scope.addSchema.remarks == null) {
$scope.addSchema.remarks = " Force Register Schema option overriding schema compatibility has been selected."
} else {
$scope.addSchema.remarks += " Force Register Schema option overriding schema compatibility has been selected."
Copy link
Contributor

@muralibasani muralibasani Oct 16, 2023

Choose a reason for hiding this comment

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

may be define the remarks in a var, and reuse in both the conditional blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is set to += if they add remarks it will be added to the end.

but if they dont set a remark it will then come out as "undefined Force...."

so if block is needed to make sure remarks are not null to give best experience to user. Also copy may change Harishini is just checking on it at the mo i think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant, if we can define a variable for the remarks string. Currently it is duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added!

aindriu-aiven and others added 10 commits October 16, 2023 11:41
…efault partition and replication numbers on a new topic request.

Signed-off-by: Aindriu Lavelle <[email protected]>
Signed-off-by: Mirjam Aulbach <[email protected]>
Signed-off-by: Mirjam Aulbach <[email protected]>
Signed-off-by: Mirjam Aulbach <[email protected]>
Signed-off-by: Aindriu Lavelle <[email protected]>
Signed-off-by: Mirjam Aulbach <[email protected]>
@programmiri programmiri force-pushed the issue-force-register-on-new-schema branch from c5ce243 to 53d4f5b Compare October 16, 2023 09:43
@aindriu-aiven aindriu-aiven marked this pull request as ready for review October 16, 2023 13:29
muralibasani
muralibasani previously approved these changes Oct 17, 2023
Copy link
Contributor

@muralibasani muralibasani left a comment

Choose a reason for hiding this comment

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

LGTM, thx

Signed-off-by: Mirjam Aulbach <[email protected]>
Copy link
Contributor

@mathieu-anderson mathieu-anderson left a comment

Choose a reason for hiding this comment

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

LGTM

@aindriu-aiven aindriu-aiven merged commit 118ab82 into main Oct 17, 2023
18 checks passed
@aindriu-aiven aindriu-aiven deleted the issue-force-register-on-new-schema branch October 17, 2023 14:27
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.

Allow user to force register a schema
4 participants