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

Release 3.0 - With or Without new Configuration? Or waiting for a more dynamic one? #1709

Closed
boaks opened this issue Aug 24, 2021 · 16 comments
Closed

Comments

@boaks
Copy link
Contributor

boaks commented Aug 24, 2021

Dear Californians,

one of the last topic of the 3.0 release in the discussions #1469 was a redesign of the NetworkConfig and DtlsConnectorConfig.
A very first variant of my ideas was published end of June (see #1648) and I merged it into the master after the 3.0.0-M3.

One critical point of that new implementation maybe the static way of using modules (see PR #1653 ). A new issue #1708 explains more details about that. For me that staticis fine, I don't see the benefits in changing it.

One outcome was also, that the API is affected by a lot of small changes, so such a redesign will require a major release. I don't see, that this could be done as alternative implementation to the old NetworkConfig and DtlsConnectorConfig.

We have now several options to go:

  • revert to the previous NetworkConfig and give those, who are interested, the time to make their contributions for the next major release (maybe based on the new Configuration).

  • use the new Configuration "as it is" (without PR Add Configuration with specific modules. #1701). With that, we benefit from the changes. It gives those, who are interested, the time to make their additional contributions for the next major release. That would unfortunately cause to adapt twice. I think, that the second adaption will be smaller, so that may be an way to go.

  • use the new Configuration with PR Add Configuration with specific modules. #1701). With that, we benefit from the changes. It gives those, who are interested, the time to make their additional contributions for the next minor release. But it's not sure, if the PR is really sufficient to implement a more dynamic way. If not it may still require a major release.

  • wait and postpone the 3.0 release to give those, who are interested, the time to make their contributions for the 3.0 release.

@boaks
Copy link
Contributor Author

boaks commented Aug 24, 2021

Option 1: revert to previous NetworkConfig and DtlsConnectorConfig.

@boaks
Copy link
Contributor Author

boaks commented Aug 24, 2021

Option 2: keep new Configuration without PR #1701

@boaks
Copy link
Contributor Author

boaks commented Aug 24, 2021

Option 3: keep new Configuration with PR #1701

@boaks
Copy link
Contributor Author

boaks commented Aug 24, 2021

Option 4: postpone release 3.0

@boaks
Copy link
Contributor Author

boaks commented Sep 10, 2021

Though I want to continue the work to have it done before mid of October, I would like to thank all for their votes.
I will work in finish PR #1701 and have it merged next week.

@boaks boaks unpinned this issue Sep 10, 2021
@sophokles73
Copy link
Contributor

With #1701 merged, it looks like we now have two alternative approaches for configuring Cf's components during application startup, right?
Is there a particular need for releasing 3.0.0 very soon, @boaks ? Maybe we can use this controversial topic to practice and improve our skills regarding open, appreciative collaboration and find a solution for 3.0.0 that everybody is happy or can at least live with :-)
@boaks, @sbernard31 are you on board?

@sbernard31
Copy link
Contributor

Let's try.

@boaks
Copy link
Contributor Author

boaks commented Sep 13, 2021

Is there a particular need for releasing 3.0.0 very soon, @boaks ?

In my opinion it's already over the time. New function requested (e.g. #1693) will for me only be possible, if we switch to 3.0.

Maybe we can use this controversial topic to practice and improve our skills regarding open, appreciative collaboration and find a solution for 3.0.0 that everybody is happy or can at least live with :-)

I'm not sure about that "collaboration". For me this was already tried. Basically I added the alternative API in order to achieve such a solution (that's PR #1701). But that approach have been rejected - requesting to remove the static one (which is my personal favorite one, already better tested). For me that sounded to be a dogma, not to use static one.

In the past we started then a "voting" to decide on topics, where otherwise no agreement could be achieved.
Usually everyone can add more explaining details to the voting or ask for it. As in the past. This time, this wasn't done, but instead the "voting" itself was criticized before closing it.

That is from my point of view then the end of collaboration. Offering an alternative API and having a voting is all, which could be done. If both is refused, I would like to ask, what should come next?

Though I go to vacation next week, we will see, how that evolves.

@boaks
Copy link
Contributor Author

boaks commented Sep 14, 2021

are you on board?

What should be the action points?
Are you both consider to spend reasonable time in working on topics?
If so, on which topics do you want to work?

@boaks
Copy link
Contributor Author

boaks commented Sep 14, 2021

One topic seems to be the current review.

Could one of you take over?

@sbernard31
Copy link
Contributor

sbernard31 commented Sep 14, 2021

My behavior seems to be considered as uncooperative and dogmatic because I asked questions about the vote and I reject an alternative solution.
I feel I need to give my perspective about that.


Firstable "vote" and "alternative solution" are not about same subject.

I have some concerns about Configuration refactoring.

  • "Vote" is about what we do waiting we solve(or get agreeement) about those concerns.
  • "alternative solution" is a attempt to fix one of the concern.

About the vote :
Unfortunately I need to add some context.

=== 18th august ===
I explain that I have concern with Configuration refactoring and more time is needed. (meaning postpone the feature or postpone the 3.0.0 release)
(eclipse-leshan/leshan#1073 (comment))

You answer :

To be frank, for me the old Configuration was not too bad. At least for 3.0 it is an alternative. I worked on the redesign, because my feeling was, that this changes the API a lot, and that's what happened. But if we feel uncomfortable with the changes it forces right now, postpone that to 4.0 will also work.
(eclipse-leshan/leshan#1073 (comment))

So I took time to dig the subject.

=== 23th august ===
I began do give more feedback :

Then I finished with :

I will be out of office in few minutes and will be back begin of September so you have time to think more abou that
(#1708 (comment))

=== 24th august ===
You create the vote : #1709

I feel its not totally surprising that I was a bit lost about what happens.
(In a more general way, I'm not sure this kind of vote is a good tool to take this kind of decision, but this is out of topic)


About the solution alternative :

One of my concerns (not all) was about the code design.
There is something in code that I consider to be problematic.
My point is to remove/replace it.
You propose to add something else aside.
So first reaction is explain that it is not my point.

You say my position is dogmatic.
This is not what I try to do.
I give arguments about my point of view, I also tried to understand your position.
I thought that a well-known good practice will be consensual enough to be a good foundation on which we could have build on an agreement. It seems that failed because that was not as consensual that I thought.

@boaks
Copy link
Contributor Author

boaks commented Sep 14, 2021

Just to make it easier for others:

There is something in code that I consider to be problematic.

That's using static for the meta-model, right?
(For others: the meta-model is a collection of type-definitions for the configuration values. e.g. CoapConfig.COAP_PORT. It's not the configuration value itself. It's used as default for generating and reading property files.)

I give arguments about my point of view, I also tried to understand your position.

That argument was mainly the "well-known good practice", without any concrete details about why the static there harms?

It seems that failed.

In my world, that static approach is working. It makes the usage of the Configuration in many cases easy. In my opinion, the world is full of false "well-known good practice". In the most cases it's more or less just a trade-off pros and cons, which the varies from use-case to use-case. I'm still confessed, that not only the staticmeta-table data is the right choice, I also have my doubts, if more flexible ways will pay off. Different meta-models, different files, I guess this will confuse people.

@Nick-The-Uncharted
Copy link

Nick-The-Uncharted commented Sep 16, 2021

Personally I do static loading all the time when it's comes to "initializing the properties unique to the whole application", especially for final variables.

What has been implemented seems okay to me (as an user). But it will be better if we keep convenience methods like org.eclipse.californium.scandium.config.DtlsConnectorConfig#setMaxConnections .

For developers I assume the new configuration api would simplify the process of adding a configuration item as we can place doc/type/"default values" together.

@boaks
Copy link
Contributor Author

boaks commented Sep 16, 2021

But it will be better if we keep convenience methods like org.eclipse.californium.scandium.config.DtlsConnectorConfig#setMaxConnections .

That was org.eclipse.californium.scandium.config.DtlsConnectorConfig.Builder#setMaxConnections or?

That works now with.

org.eclipse.californium.scandium.config.DtlsConnectorConfig.Builder#set(DtlsConfig.DTLS_MAX_CONNECTIONS, 1000);

The setter is generic and works with all Definitions. In that way, we reduce the code-copies.
Once we came to a final decision, I will ask to also remove the specific getters and replace them by a generic one. But that is no must, it's just an option.

@boaks
Copy link
Contributor Author

boaks commented Sep 16, 2021

Personally I do static loading all the time when it's comes to "initializing the properties unique to the whole application", especially for final variables.

And, if that simple, easy an mature way doesn't work for someone, the more dynamic way is possible, it must be tested, but it is possible.

@boaks
Copy link
Contributor Author

boaks commented Oct 14, 2021

The 3.0.0-RC1 is build using option 3.

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

No branches or pull requests

4 participants