Add: _uuid module by rimi0108 · Pull Request #3974 · RustPython/RustPython

rimi0108 · 2022-07-26T15:05:47Z

Add _uuid module
Add _uuid function generate_time_safe()
uuid v1 = generate_time_safe
Return mac address to node_id if you have mac address or return random generated bytes to node_id if you do not have mac address

youknowone · 2022-07-26T15:08:00Z

stdlib/Cargo.toml

 num_enum = "0.5.7"
 ascii = "1.0.0"
+once_cell = "1.13.0"
+mac_address = "1.1.3"


Adding this line instead of dependencies.uuid section
will be more easy to read

uuid = { version = "1.1.2", features = ["v1", "fast-rng", "macro-diagnostics"] }

mac_address needs more cfg guard for more OS.
See mmap case. By referring CI failure, we (at least) need to exclude iOS and android.

youknowone · 2022-07-26T15:08:37Z

stdlib/src/lib.rs

 mod ssl;
 #[cfg(all(unix, not(target_os = "redox")))]
 mod termios;
+#[cfg(target_os = "macos")]


what's the blocker for linux and windows?

youknowone · 2022-07-26T15:08:50Z

stdlib/src/uuid.rs

+
+#[pymodule]
+mod _uuid {
+


Suggested change

youknowone · 2022-07-26T15:11:08Z

stdlib/src/uuid.rs

+        match get_mac_address() {
+            Ok(Some(_ma)) => {
+                let node_id = NODE_ID.get_or_init(|| get_mac_address().unwrap().unwrap().bytes());
+                node_id
+            }
+            Ok(None) => {
+                let node_id = NODE_ID.get_or_init(|| rand::thread_rng().gen::<[u8; 6]>());
+                node_id
+            }
+            Err(_e) => {
+                let node_id = NODE_ID.get_or_init(|| rand::thread_rng().gen::<[u8; 6]>());
+                node_id
+            }
+        }


this is checking get_mac_address every time. That probably is not expectation for node id. NODE_ID.get_or_init must be the outmost wrapper of this function

youknowone · 2022-07-27T01:38:52Z

_uuid.has_uuid_generate_time_safe seems required to let the module run

youknowone · 2022-07-27T12:41:42Z

stdlib/src/uuid.rs

+            Ok(None) => {
+                let node_id = rand::thread_rng().gen::<[u8; 6]>();
+                node_id
+            }
+            Err(_e) => {
+                let node_id = rand::thread_rng().gen::<[u8; 6]>();
+                node_id
+            }


Suggested change

Ok(None) => {

let node_id = rand::thread_rng().gen::<[u8; 6]>();

node_id

}

Err(_e) => {

let node_id = rand::thread_rng().gen::<[u8; 6]>();

node_id

}

_ => {

let node_id = rand::thread_rng().gen::<[u8; 6]>();

node_id

}

youknowone · 2022-07-27T12:42:39Z

stdlib/src/uuid.rs

+        let now = now_unix_duration();
+        static CONTEXT: Context = Context::new(0);


Suggested change

let now = now_unix_duration();

static CONTEXT: Context = Context::new(0);

static CONTEXT: Context = Context::new(0);

let now = now_unix_duration();

technically this is ok but I feel uncomfortable to have static below without a good reason

youknowone · 2022-07-27T12:45:10Z

stdlib/src/uuid.rs

+        static CONTEXT: Context = Context::new(0);
+        let ts = Timestamp::from_unix(&CONTEXT, now.as_secs(), now.subsec_nanos());
+
+        let node_id = get_node_id();


If we will not use the value from second call, why do we call this function every time?
You can move this line in get_or_init().

youknowone · 2022-07-27T12:45:35Z

stdlib/src/uuid.rs

+    }
+
+    #[pyfunction]
+    fn generate_time_safe(_vm: &VirtualMachine) -> (Vec<u8>, PyNone) {


Suggested change

fn generate_time_safe(_vm: &VirtualMachine) -> (Vec<u8>, PyNone) {

fn generate_time_safe() -> (Vec<u8>, PyNone) {

youknowone · 2022-07-27T12:46:06Z

stdlib/src/uuid.rs

+    }
+
+    #[pyattr]
+    fn has_uuid_generate_time_safe(_vm: &VirtualMachine) -> PyInt {


Suggested change

fn has_uuid_generate_time_safe(_vm: &VirtualMachine) -> PyInt {

fn has_uuid_generate_time_safe() -> u32 {

youknowone · 2022-07-27T12:49:01Z

stdlib/src/uuid.rs

+
+    #[pyattr]
+    fn has_uuid_generate_time_safe(_vm: &VirtualMachine) -> PyInt {
+        PyInt::from(0)


This is interesting. What this value is meaning? 🤔

I wrote the code like that because it was printed below the cpython
>>> _uuid.has_uuid_generate_time_safe
0

youknowone

You seem didn't set git email correctly. Check git log and if it matches your actual email (and github email)

youknowone · 2022-07-31T22:42:33Z

stdlib/Cargo.toml

 num_enum = "0.5.7"
 ascii = "1.0.0"
+once_cell = "1.13.0"
+mac_address = "1.1.3"


mac_address needs more cfg guard for more OS.
See mmap case. By referring CI failure, we (at least) need to exclude iOS and android.

youknowone

looks good, thank you for long time effort

Add: _uuid module

a6eb29d

youknowone requested changes Jul 26, 2022

View reviewed changes

Hyerim Choi added 3 commits July 27, 2022 00:16

Fix: change dependency uuid line

244d9e5

Fix: delete line

722217f

Fix: change NODE_ID.get_or_init to outer wrapper of get_node_id function

841a46e

Fix: remove #[cfg(target_os = macos)]

d2e5e72

youknowone added the z-ca-2022 Tag to track contrubution-academy 2022 label Jul 27, 2022

Hyerim Choi added 2 commits July 27, 2022 20:56

Add: _uuid.has_uuid_generate_time_safe

3693347

Fix: cargo fmt

9aae0ff

youknowone requested changes Jul 27, 2022

View reviewed changes

Hyerim Choi added 9 commits July 27, 2022 21:52

Fix: delete unecessary let node_id

de2d2b2

Fix: delete generate_time_safe parameter vm

c984ad8

Fix: Change static location

22bb5c1

Fix: match get_mac_address()

5e38eb4

Fix: move get_node_id function in get_or_init().

6ebf39f

Fix: PyInt to u32

cbeed54

Fix: target os

af3f87e

Add: not(target_arch = wasm32),)

4f67273

Fix: cargo clippy error

920b53d

youknowone requested changes Jul 31, 2022

View reviewed changes

rimi0108 added 5 commits August 4, 2022 19:55

Fix: except for ios, android

ea4efa9

Fix: cfg ios, android all to any

c8a44bd

Add: exclude Web Assembly

2a05b68

Add: exclude target os windows

0d1b5bb

Fix: cargo fmt

bba6746

youknowone approved these changes Aug 7, 2022

View reviewed changes

youknowone merged commit 42290b1 into RustPython:main Aug 7, 2022

		let now = now_unix_duration();
		static CONTEXT: Context = Context::new(0);

	fn generate_time_safe(_vm: &VirtualMachine) -> (Vec<u8>, PyNone) {
	fn generate_time_safe() -> (Vec<u8>, PyNone) {

	fn has_uuid_generate_time_safe(_vm: &VirtualMachine) -> PyInt {
	fn has_uuid_generate_time_safe() -> u32 {

Conversation

rimi0108 commented Jul 26, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowone commented Jul 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rimi0108 Jul 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rimi0108 Jul 28, 2022 •

edited

Loading