Skip to content

[CIAM-2500/2519] Remove SR from behind RBAC Stage 3 #1578

Merged
Brian Strauch (brianstrauch) merged 4 commits intomainfrom
lfan/ciam-2500-2519
Dec 8, 2022
Merged

[CIAM-2500/2519] Remove SR from behind RBAC Stage 3 #1578
Brian Strauch (brianstrauch) merged 4 commits intomainfrom
lfan/ciam-2500-2519

Conversation

@lucy-fan
Copy link
Member

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
    • yes: ok
    • no: DO NOT MERGE until the required functionalites are live in prod

What

Moved #1567 to main branch and added flag back in to keep KSQL code under the flag.

References

Test & Review

Co-authored-by: Brian Strauch <bstrauch@confluent.io>
Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm

@lucy-fan
Copy link
Member Author

Brian Strauch (@brianstrauch) It looks like some KSQL create tests are failing with test panicked: runtime error: invalid memory address or nil pointer dereference when we call FetchSchemaRegistryByAccountId during ksql create to see if there is an existing SR cluster so we know whether to print the warning message or not.

Would you happen to know why these tests are failing? I've done a bit of debugging but can't seem to figure out the root cause. The earlier PR with v3 as the base branch didn't seem to have a problem with this :(

@brianstrauch

Lucy Fan (@lucy-fan) The output of a panic() will show the line number where the nil pointer occurs. In this case, we were just missing a reference to the Schema Registry client.

@lucy-fan
Copy link
Member Author

Brian Strauch (@brianstrauch) Thanks for all your help so far! This PR is pretty much all ready to go except for this new check I've never seen before, ci/semaphore-onprem/pr: Confluent CLI. Is there any way to see what this check is doing / the progress of this check like the other checks?

@brianstrauch

Lucy Fan (@lucy-fan) Looks like it was related to this Slack message, but it's been reverted (and your tests are all passing!) https://confluent.slack.com/archives/C038ZJ00P/p1670452555457079?thread_ts=1667599823.545889&cid=C038ZJ00P

@lucy-fan
Copy link
Member Author

Brian Strauch (@brianstrauch) Ok great, thanks. I can’t merge this PR though as it’s marked as required. Is there any way to bypass this?

@brianstrauch

Lucy Fan (@lucy-fan) Ah, I can merge it as an administrator!

@brianstrauch Brian Strauch (brianstrauch) deleted the lfan/ciam-2500-2519 branch December 8, 2022 20: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.

3 participants