Attention is currently required from: dexter, fixeria, laforge, neels.
Hello Jenkins Builder, dexter, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/39742?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Verified+1 by Jenkins Builder
Change subject: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi
......................................................................
[2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi
Main points/rationales of the refactoring, details below:
1) common validation implementation
2) offer classmethods
The new features are optional, and will be heavily used by batch
personalization patches coming soon.
Implement Iccid and Imsi to use the new way, with a common abstract
DecimalParam implementation.
So far leave the other parameter classes working as they always did, to
follow suit in subsequent commits.
Details:
1) common validation implementation:
There are very common validation steps in the various parameter
implementations. It is more convenient and much more readable to
implement those once and set simple validation parameters per subclass.
So there now is a validate_val() classmethod, which subclasses can use
as-is to apply the validation parameters -- or subclasses can override
their cls.validate_val() for specialized validation.
(Those subclasses that this patch doesn't touch still override the
self.validate() instance method. Hence they still work as before this
patch, but don't use the new common features yet.)
2) offer stateless classmethods:
It is useful for...
- batch processing of multiple profiles (in upcoming patches) and
- user input validation
to be able to have classmethods that do what self.validate() and
self.apply() do, but do not modify any self.* members.
So far the paradigm was to create a class instance to keep state about
the value. This remains available, but in addition we make available the
paradigm of a singleton that is stateless (the classmethods).
Using self.validate() and self.apply() still work the same as before
this patch, i.e. via self.input_value and self.value -- but in addition,
there are now classmethods that don't touch self.* members.
Related: SYS#6768
Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8
---
M pySim/esim/saip/personalization.py
1 file changed, 180 insertions(+), 34 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/42/39742/8
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8
Gerrit-Change-Number: 39742
Gerrit-PatchSet: 8
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Attention is currently required from: fixeria, laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/39741?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: [1/6] personalization: refactor: drop ClassVarMeta use
......................................................................
[1/6] personalization: refactor: drop ClassVarMeta use
Drop the ClassVarMeta/metaclass/ABCMeta stuff -- it doesn't seem to
serve any purpose that is not similarly achieved with plain python
inheritance.
Upcoming patches will use normal inheritance a lot more.
Note that most use of the ClassVarMeta was in the SdKey subclasses, and
that these currently don't actually work. See the fix in patch
I07dfc378705eba1318e9e8652796cbde106c6a52 .
name: set a default name from the python class, as ClassVarMeta did.
Also allow setting an explicit string as name instead, per subclass
implementation (see I31f390d634e58c384589c50a33ca45d6f86d4e10).
Related: SYS#6768
Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051
---
M pySim/esim/saip/personalization.py
1 file changed, 149 insertions(+), 82 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/41/39741/8
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051
Gerrit-Change-Number: 39741
Gerrit-PatchSet: 8
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: fixeria, laforge.
neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email )
Change subject: [1/6] personalization: refactor: drop ClassVarMeta use
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS6:
> CSV matching: I have other use cases than you do. […]
In this new patch set, the name and camel_to_snake() are back (they were lost by accident from branch rebasing).
reasons in my decision against a meta class:
- i don't understand what purpose a meta class serves.
- fixeria's example shows that the shortest way to achieve class attributes is
~~~
class Foo:
a = 1
b = 2
~~~
I like this plain python better than kwargs juggling.
- upcoming patches add a lot of class attributes, and i would like to consistently use one way. I chose the "normal python" way, so as a first step, this patch drops the meta class.
- name: i would like to set explicit names like 'IMSI' instead of 'imsi',
Hence i change to using an explicit 'name' attribute instead of getattr magic.
When no explicit name is set, all still behaves as before this patch.
I hope that I've made my point, so i am now, in a hopeful way, resolving this thread.
Please -1 again if i should change it.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051
Gerrit-Change-Number: 39741
Gerrit-PatchSet: 7
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 19 Apr 2025 20:59:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: fixeria, laforge.
neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email )
Change subject: [1/6] personalization: refactor: drop ClassVarMeta use
......................................................................
Patch Set 7:
(1 comment)
File pySim/esim/saip/personalization.py:
https://gerrit.osmocom.org/c/pysim/+/39741/comment/2f8fe753_c0f31009?usp=em… :
PS7, Line 155: class SdKeyScp80_01(SdKey):
> I'm still somewhat doubtful that this is an improvement. […]
(merging with the other "thread")
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051
Gerrit-Change-Number: 39741
Gerrit-PatchSet: 7
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 19 Apr 2025 20:37:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>