Skip to content

#863 Fix naming of consumer group attribute#870

Merged
mtmk merged 1 commit into
nats-io:release/2.7from
thompson-tomo:fixes/#863_FixAttributeName
May 27, 2025
Merged

#863 Fix naming of consumer group attribute#870
mtmk merged 1 commit into
nats-io:release/2.7from
thompson-tomo:fixes/#863_FixAttributeName

Conversation

@thompson-tomo

Copy link
Copy Markdown
Contributor

Closes #863

@thompson-tomo
thompson-tomo force-pushed the fixes/#863_FixAttributeName branch from 9f7210c to aecea64 Compare May 24, 2025 04:38

@mtmk mtmk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@thompson-tomo

Copy link
Copy Markdown
Contributor Author

Flaky test again?

@mtmk

mtmk commented May 24, 2025

Copy link
Copy Markdown
Member

Flaky test again?

yep! just kicked it

@thompson-tomo

Copy link
Copy Markdown
Contributor Author

And all passing. 🎉

@mtmk

mtmk commented May 24, 2025

Copy link
Copy Markdown
Member

ouch 😵

image

@thompson-tomo sorry didn't warn you about this! how's your git-fu? i usually suggest to squash all into one commit, sign and force push

@thompson-tomo

Copy link
Copy Markdown
Contributor Author

I have fixed my commits but it appears that there is something wrong with either e59beee or 56b4095 as branches which include them are running into this issue but branches from earlier are not. That is why #869 is also in error

@thompson-tomo
thompson-tomo force-pushed the fixes/#863_FixAttributeName branch from d899d69 to db6536a Compare May 25, 2025 02:31
@mtmk

mtmk commented May 25, 2025

Copy link
Copy Markdown
Member

I have fixed my commits but it appears that there is something wrong with either e59beee or 56b4095 as branches which include them are running into this issue but branches from earlier are not. That is why #869 is also in error

yes, those two commits were added to main after you created your branches. you need to either rebase or merge them once you sign your commits. just make sure your diff here is correct and your commits are signed (with GPG or SSH). hit me on https://slack.nats.io if you want i'm not a git expert but i should be able to help you with this 🤞😅

@thompson-tomo
thompson-tomo force-pushed the fixes/#863_FixAttributeName branch from db6536a to 9106417 Compare May 25, 2025 13:57
@mtmk

mtmk commented May 25, 2025

Copy link
Copy Markdown
Member

sorry @thompson-tomo i think i didn't explain properly. we need the commits to be signed as in here https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification using gpg (i believe you can use ssh as well). afaik we don't necessarily need the 'signed off by' text.

@thompson-tomo
thompson-tomo force-pushed the fixes/#863_FixAttributeName branch from 9106417 to 94d13e7 Compare May 25, 2025 14:23
Signed-off-by: James Thompson <thompson.tomo@outlook.com>
@thompson-tomo
thompson-tomo force-pushed the fixes/#863_FixAttributeName branch from 94d13e7 to b3cde1c Compare May 25, 2025 14:31
@thompson-tomo

thompson-tomo commented May 25, 2025

Copy link
Copy Markdown
Contributor Author

And i have resolved it on 1 of my computers. :D I was missing the gpg key setup part, as i'd never encountered a project requiring that to have been setup Usually if anything having the signed off part + a signed cla is usually sufficient.

Note the checks for the 2.7 branch are different to the main so that also threw me.

Thanks for your support.

@mtmk

mtmk commented May 25, 2025

Copy link
Copy Markdown
Member

And i have resolved it on 1 of my computers. :D I was missing the gpg key setup part, as i'd never encountered a project requiring that to have been setup Usually if anything having the signed off part + a signed cla is usually sufficient.

yea it's a CNCF thing we need to have GPG signed commits.

Note the checks for the 2.7 branch are different to the main so that also threw me.

I'll check that, thanks.

Thanks for your support.

you're welcome. sorry for the hassle, i should've noticed it before and you have a few commits across few PR 😅 hopefully didn't cause you too much work. thanks!

@thompson-tomo

Copy link
Copy Markdown
Contributor Author

yea it's a CNCF thing we need to have GPG signed commits.

Interesting as projects like cloud events (also cncf) just have dco.

@mtmk

mtmk commented May 25, 2025

Copy link
Copy Markdown
Member

yea it's a CNCF thing we need to have GPG signed commits.

Interesting as projects like cloud events (also cncf) just have dco.

hmmm ok. i thought it was cncf. maybe it's nats-io specific thing not sure tbh. it's a 'good thing' i agree in general. it's just like you said if we can make it so new contributions are made aware as quick as possible.

@mtmk
mtmk changed the base branch from main to release/2.7 May 27, 2025 11:43
@mtmk
mtmk merged commit b09e92c into nats-io:release/2.7 May 27, 2025
@thompson-tomo
thompson-tomo deleted the fixes/#863_FixAttributeName branch May 27, 2025 11:44
mtmk added a commit that referenced this pull request Jul 18, 2025
* #851 Move Serialization Interface into Abstractions (#858)
* Obsolete ReplyAsync method for NatsJsMsg (#839)
* Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)
* #848 Tweak dependencies (#853)
* #863 Fix naming of consumer group attribute (#870)
@mtmk mtmk mentioned this pull request Jul 18, 2025
mtmk added a commit that referenced this pull request Jul 18, 2025
* #851 Move Serialization Interface into Abstractions (#858)
* Obsolete ReplyAsync method for NatsJsMsg (#839)
* Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)
* #848 Tweak dependencies (#853)
* #863 Fix naming of consumer group attribute (#870)
@mtmk mtmk mentioned this pull request Sep 22, 2025
mtmk added a commit that referenced this pull request Dec 19, 2025
* #863 Fix naming of consumer group attribute (#870)

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* #848 Tweak dependencies (#853)

* #848 Tweak dependencies

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Update NATS.Client.Core.csproj

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Update NATS.Client.Core.csproj

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Make STJ explicit dependency for JetStream

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Update NATS.Client.JetStream.csproj

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Force sdk to 8.0.0

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Add newer stj for net 6

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

---------

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)

* Add support for `Filter` and `Enrich` for OpenTelemetry activities

* Make `internal` methods in `internal Telemetry` `public`

* Fix package versions and whatnot

* Remove `TracerProviderBuilderExtensions`

* Include `Deserialize` in the receive activity

* Revert back accidental change

* Add `ParentContext` to `NatsInstrumentationContext`

* Make `GetActivityContext` public to provide the ability to get context activity context

* Make preprocessor directive more accurate

* Revert .csproj formatting

* Move public artifacts out of the `Internal` namespace/folder

* Fix build script

---------

Co-authored-by: Ziya Suzen <ziya@suzen.net>

* Obsolete ReplyAsync method for NatsJsMsg (#839)

* Obsolete ReplyAsync method

* Obsolete ReplyAsync method on interface

* #851 Move Serialization Interface into Abstractions (#858)

* Tweaks

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Add missing dependencies

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* switch to system.memory

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Added missing using

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Reduce csproj file contents

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

---------

Signed-off-by: James Thompson <thompson.tomo@outlook.com>

* Release 2.7.0-preview.1 (#905)

* #851 Move Serialization Interface into Abstractions (#858)
* Obsolete ReplyAsync method for NatsJsMsg (#839)
* Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)
* #848 Tweak dependencies (#853)
* #863 Fix naming of consumer group attribute (#870)

* fix: `NatsInstrumentationOptions.Default` gets reset each time (#907)

* initialize `NatsInstrumentationOptions.Default` once

* `NatsInstrumentationContext.ActivityContext` doesn't need to be nullable

* no need for `GetActivityContext` on NatsJSMsg

* Release 2.7.0-preview.2 (#908)

* fix: `NatsInstrumentationOptions.Default` gets reset each time (#907)
* Also, fixes from main

* Fix build warnings (#912)

* fix: `NatsJSConsumer` never disposing receive activities (#911)

* Fix `NatsJSConsumer` never disposing receive activities

* Use `ActivityEndingMsgReader` to `NatsJSFetch` and `NatsJSOrderedConsume`

* Consolidate activity ending message readers into one class

* Run `dotnet format`

* Release 2.7.0-preview.3 (#922)

* fix: `NatsJSConsumer` never disposing receive activities (#911)
* Merge from main

* chore: rework NatsOpt.Default initialization (#921)

* chore: rework NatsOpt.Default initialization

* chore: removed unused using statement

* Fix kv ttl interface (#909)

KV TTL should only be allowed on Create and Purge

* Release 2.7.0-preview.4 (#931)

Fixes merged from main

* Release 2.7.0-preview.5 (#944)

Merge from main aligning with 2.6.8 release.

* Fix JetStream publish retry defaults (#939)

Decision to retry jetstream publish requests should be letft to the
application since it depends on the delivery and durability
requirements of their solution.

* Fix publish 503 test (#958)

With 2.7.x we have made the default to not re-publish on failure.

* Release 2.7.0-preview.6 (#956)

* Fix JetStream publish retry defaults (#939)
* Merge from main

* Release 2.7.0-preview.7 (#970)

Merged from main

* fix: handle 408 Requests Pending responses for fetch requests (#973)

* Handle 408 Requests Pending responses for fetch requests

* Remove redundant `Console.WriteLine` that was put there for testing

* Release 2.7.0-preview.8 (#986)

* fix: handle 408 Requests Pending responses for fetch requests (#973)
* (merge from main) Object store item size fix (#977)

* Fix Ad-Hoc JSON Serializer to use Default Options (#984)

* Fix connection state for consume (#959)

* Fix connection state for consume

* Enhance connection state handling with NatsConnectionFailedException

* Improve error handling for connection failures and add support for configurable 503 error thresholds in JetStream consumers

* Add tests for connection failure handling and configurable 503 error thresholds in JetStream consumers

* Fix format

* Fix test

* Update INatsJsConsumer to return INatsJsMsg (#1004)

---------

Signed-off-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: Arad Alvand <aradalvand@gmail.com>
Co-authored-by: Yeong Jong Lim <46077396+yeongjonglim@users.noreply.github.com>
Co-authored-by: regnrat <66414875+regnrat@users.noreply.github.com>
mtmk added a commit that referenced this pull request Dec 19, 2025
* Replace `OperationCanceledException` with `NatsTimeoutException` (#1022)
* Replace deprecated IndexRange package with Microsoft.Bcl.Memory to solve possible dependency conflicts (for netFramework/netStandard2.0) (#1009)
* PeerInfo.Lag is wrong type (#1021)
* (Merge Branch) Release 2.7 (#874)
  * Update INatsJsConsumer to return INatsJsMsg (#1004)
  * Fix connection state for consume (#959)
  * Fix Ad-Hoc JSON Serializer to use Default Options (#984)
  * fix: handle 408 Requests Pending responses for fetch requests (#973)
  * Fix publish 503 test (#958)
  * Fix JetStream publish retry defaults (#939)
  * Fix kv ttl interface (#909)
  * chore: rework NatsOpt.Default initialization (#921)
  * fix: `NatsJSConsumer` never disposing receive activities (#911)
  * Fix build warnings (#912)
  * fix: `NatsInstrumentationOptions.Default` gets reset each time (#907)
  * #851 Move Serialization Interface into Abstractions (#858)
  * Obsolete ReplyAsync method for NatsJsMsg (#839)
  * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)
  * #848 Tweak dependencies (#853)
  * #863 Fix naming of consumer group attribute (#870)
@mtmk mtmk mentioned this pull request Dec 19, 2025
mtmk added a commit that referenced this pull request Dec 19, 2025
* Replace `OperationCanceledException` with `NatsTimeoutException` (#1022)
* Replace deprecated IndexRange package with Microsoft.Bcl.Memory to solve possible dependency conflicts (for netFramework/netStandard2.0) (#1009)
* PeerInfo.Lag is wrong type (#1021)
* (Merge Branch) Release 2.7 (#874)
  * Update INatsJsConsumer to return INatsJsMsg (#1004)
  * Fix connection state for consume (#959)
  * Fix Ad-Hoc JSON Serializer to use Default Options (#984)
  * fix: handle 408 Requests Pending responses for fetch requests (#973)
  * Fix publish 503 test (#958)
  * Fix JetStream publish retry defaults (#939)
  * Fix kv ttl interface (#909)
  * chore: rework NatsOpt.Default initialization (#921)
  * fix: `NatsJSConsumer` never disposing receive activities (#911)
  * Fix build warnings (#912)
  * fix: `NatsInstrumentationOptions.Default` gets reset each time (#907)
  * #851 Move Serialization Interface into Abstractions (#858)
  * Obsolete ReplyAsync method for NatsJsMsg (#839)
  * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859)
  * #848 Tweak dependencies (#853)
  * #863 Fix naming of consumer group attribute (#870)
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.

Rename consumer group attribute

2 participants