Skip to content

Adding int identifier for transactional ID#4803

Closed
John Mille (JohnPreston) wants to merge 3 commits intoconfluentinc:masterfrom
JohnPreston:feature/transactional-id-int
Closed

Adding int identifier for transactional ID#4803
John Mille (JohnPreston) wants to merge 3 commits intoconfluentinc:masterfrom
JohnPreston:feature/transactional-id-int

Conversation

@JohnPreston
Copy link
Copy Markdown
Contributor

@JohnPreston John Mille (JohnPreston) commented Aug 5, 2024

Adding the int ID for transactional ID, to enable using that as a ResourceType in the clients (python being my use-case specifically)

@JohnPreston John Mille (JohnPreston) requested a review from a team as a code owner August 5, 2024 11:10
@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented Aug 5, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

This was referenced Aug 5, 2024
Closed
Open
@pranavrth
Copy link
Copy Markdown
Member

Pranav Rathi (pranavrth) commented Aug 8, 2024

/sem-approve

pranavrth
Pranav Rathi (pranavrth) reviewed Aug 8, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@pranavrth Pranav Rathi (pranavrth) left a comment

Choose a reason for hiding this comment

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

Style check and tests are failing. Please fix those.

To fix style check, run make style-check-changed for the changed code. Note that the change code is uncommitted code.

Comment thread src/rdkafka.h Outdated
@JohnPreston
Copy link
Copy Markdown
Contributor Author

John Mille (JohnPreston) commented Aug 12, 2024 •
edited
Loading

/sem-approve
Well that was worth a shot :D
Pranav Rathi (@pranavrth) can you please re-trigger the tests?

@pranavrth
Copy link
Copy Markdown
Member

Pranav Rathi (pranavrth) commented Aug 21, 2024

/sem-approve

@JohnPreston
Copy link
Copy Markdown
Contributor Author

John Mille (JohnPreston) commented Aug 21, 2024

I mean, the style check I ran the command to fix that locally so I don't know what else to do here :/
I am not sure why the tests pass on some architecture but not with others. Given that the validation test is to check the value as per a statically expected set of returns, I am not sure how that comes to be.

Any suggestions?

@pranavrth
Copy link
Copy Markdown
Member

Pranav Rathi (pranavrth) commented Aug 22, 2024 •
edited
Loading

The style check works with uncommitted and unstaged changes. Can you try to uncommit your changes and then run make style-fix-changed and then force push the changes to this branch after committing.

Test still fails. Failing on different architecture is not related to the test cases. Its something else that I can check later when the style check and tests passes. Please focus on the Linux Ubuntu x64: source build and Style check jobs in the CI.

Screenshot 2024-08-22 130127

@pranavrth Pranav Rathi (pranavrth) mentioned this pull request Sep 30, 2024
Merged
@pranavrth
Copy link
Copy Markdown
Member

Pranav Rathi (pranavrth) commented Sep 30, 2024

Closing this PR.
Continued in #4856

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

Reviewers

@pranavrth Pranav Rathi (pranavrth) Awaiting requested review from pranavrth

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JohnPreston @pranavrth