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

Allow to use custom keys in RedisRegistrationStore #1409

Closed

Conversation

aliakseiz
Copy link
Contributor

My attempt to implement requirements mentioned in #1249

Not sure about the prefixes, though. Left them untouched for now, but suggestions are still welcome.

Copy link
Contributor

@sbernard31 sbernard31 left a comment

Choose a reason for hiding this comment

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

I will try to propose some javadoc for builder setter in another review

Copy link
Contributor

@sbernard31 sbernard31 left a comment

Choose a reason for hiding this comment

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

My separated review about javadoc

@sbernard31
Copy link
Contributor

Not sure about the prefixes, though. Left them untouched for now, but suggestions are still welcome.

That sounds good to me 👍

Maybe out of topic but :
Reading this PR, and especially this builder method :
public Builder setRegistrationSerDes(RegistrationSerDes registrationSerDes)

I was thinking that maybe we should also have a setter to serialize/de-serialize Observation ?
public Builder setObservationSerDes(ObservationSerDes observationSerDes)

@sbernard31
Copy link
Contributor

Could you also rebase your PR on master because I fixed a github action issue : #1410. 🙏

@sbernard31
Copy link
Contributor

@aliakseiz let me know when you plan to finalize this because I would like to declare a code freeze on master soon because of M11 release.
Potentially, If you can not spend more time on it, I can finished work on this PR. (most is already done)

@sbernard31
Copy link
Contributor

I will start to work on this.

@sbernard31
Copy link
Contributor

@aliakseiz I created a new PR fixing comment above. See #1414.

I plan to integrate it into master before the end of the day.
If you have time, you could review it.
(If you can not, it's not a be deal, sorry about the rush this is pretty unusual 😞)

@aliakseiz
Copy link
Contributor Author

@sbernard31 too bad I only saw your comments today.
Thanks for completing this PR anyway.

@sbernard31
Copy link
Contributor

This PR is part of #1414 and is now integrated in master.

@aliakseiz thx a lot for your contributions 🙏 !

@sbernard31 sbernard31 closed this Mar 8, 2023
@sbernard31
Copy link
Contributor

@aliakseiz could I ask which part and how you are currently using Leshan ?

Would you like to add your company as Leshan adopters ? (see #830)

@aliakseiz
Copy link
Contributor Author

@sbernard31 we have a LwM2M gateway heavily based on the Leshan, which imports core and server packages.
Major differences relate to SecurityServlet, observation logic and, till recently, to Redis storage. Several thousands of devices both secure and insecure are connected to it. Sending measurements primarily over UDP.

We also have multiple variations of LwM2M clients, that are based on Leshan and Wakaama.

Would you like to add your company as Leshan adopters ? (see #830)

That would be very nice actually. I will leave a comment there.

@sbernard31
Copy link
Contributor

@aliakseiz thx for details explanation.

That would be very nice actually. I will leave a comment there.

That would be great. 👍
Once you add the comment at #830 and if you agree I will create a PR at eclipsefdn-project-adopters to add your company at Leshan adopters.

@aliakseiz
Copy link
Contributor Author

Actually I already did: https://gitlab.eclipse.org/eclipsefdn/it/api/eclipsefdn-project-adopters/-/issues/193

Does it still make sense to leave a comment in #830?

@sbernard31
Copy link
Contributor

Actually I already did: https://gitlab.eclipse.org/eclipsefdn/it/api/eclipsefdn-project-adopters/-/issues/193

Oops I missed that 😁

Does it still make sense to leave a comment in #830?

It makes sense but clearly not mandatory.

Some users add both.
Some only add to Leshan adopters.
Some are OK to put comment in PR but doesn't want to be in Leshan adopters.

All is fine too me.

@sbernard31
Copy link
Contributor

(I created a PR for it : https://gitlab.eclipse.org/eclipsefdn/it/api/eclipsefdn-project-adopters/-/merge_requests/203)

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.

2 participants