chore: Request status tracking #18

Raalsky · 2024-08-20T14:58:52Z

No description provided.

kgodlewski · 2024-08-21T11:45:56Z

src/neptune_scale/__init__.py

+            if not self._sync_process.is_alive():
+                if verbose:
+                    logger.warning("Sync process is not running")
+                return  # No need to wait if the sync process is not running


If sync process is dead, shouldn't we signal an error? It's not situation normal, and we have no means of restarting it (and we shouldn't)

We will (separate PR that should rework the signal handling)

kgodlewski · 2024-08-21T12:00:27Z

src/neptune_scale/core/components/sync_process.py

+                return None
+
+            items = []
+            for i in range(min(size, min(max_size, size))):


min(size, max_size), or did you mean something different?

Ohh, good catch. I'm blaming Copilot as I was certain that it was range(min(size, max_size)) before some refactors.

src/neptune_scale/core/components/sync_process.py

Raalsky · 2024-08-21T21:11:49Z

src/neptune_scale/__init__.py

+        )
+
+
+def should_print_message(last_message_printed: Optional[float]) -> bool:


@kgodlewski I've done the refactor requested by you in previous PR

Is this user-facing? If so, what's the purpose?

It is not 😉

normandy7 · 2024-08-22T11:13:59Z

src/neptune_scale/__init__.py

+        )
+
+
+def should_print_message(last_message_printed: Optional[float]) -> bool:


Is this user-facing? If so, what's the purpose?

src/neptune_scale/__init__.py

src/neptune_scale/exceptions.py

normandy7 · 2024-08-22T11:31:43Z

src/neptune_scale/exceptions.py

+{h1}
+----NeptuneRunDuplicate--------------------------------------------------------
+{end}
+Identical run already exists.


This is raised when you create a run with an existing ID but without resume=True?

Yes.
This will trigger error above:

with Run(run_id=run_id, family=run_id): ... with Run(run_id=run_id, family=run_id): ...

This works:

with Run(run_id=run_id, family=run_id): ... with Run(run_id=run_id, family=run_id, resume=True): ...

src/neptune_scale/exceptions.py

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

kgodlewski · 2024-08-22T12:34:46Z

src/neptune_scale/__init__.py

                else:
-                    if last_message_printed is None or time.time() - last_message_printed > STOP_MESSAGE_FREQUENCY:
+                    if verbose and should_print_message(last_message_printed):


tiny detail, but in the future we could do:

def print_message(msg: str, last_print_timestamp: Optional[float], verbose: bool) -> float, the func would return the current time if it printed the message. So we could just do:

last_message_printed = print_message(f"some message", last_message_printed, verbose)

kgodlewski · 2024-08-22T12:38:44Z

src/neptune_scale/core/components/sync_process.py

+                items.append(item)
+            return items
+
+    def commit(self, n: int) -> None:


Not just now, but we need a different name here, don't know just yet how different ;)

I'm actually making it even more common name (introduced commit to all of the threads); let's address that in next PR to get a better context

kgodlewski · 2024-08-22T12:40:16Z

src/neptune_scale/core/components/sync_process.py

-        return MockedApiClient()
-    return HostedApiClient(api_token=api_token)
-
-
 class SyncThread(Daemon, WithResources):


How about renaming this to SenderThread, or ApiSenderThread, or something that is less generic than Sync?

SenderThread works for me

kgodlewski · 2024-08-22T12:46:04Z

src/neptune_scale/core/components/sync_process.py

@@ -283,7 +400,11 @@ def work(self) -> None:
            try:
                run_operation = RunOperation()
                run_operation.ParseFromString(data)
-                self.submit(operation=run_operation)
+                request_id = self.submit(operation=run_operation)


Should we ever return None from self.submit -> Reponse.parsed? I mean, if the backend fails to return a proper RequestId, I think we should raise an error

Let's a address that in next PR

Raalsky mentioned this pull request Aug 21, 2024

Sending operations asynchronously #17

Merged

kgodlewski reviewed Aug 21, 2024

View reviewed changes

Raalsky force-pushed the rj/basic-submission-flow branch from a89c60f to e9659f3 Compare August 21, 2024 14:02

Base automatically changed from rj/basic-submission-flow to main August 21, 2024 16:59

Requests tracking

7e77204

Raalsky force-pushed the rj/requests-tracking branch from 3d68f02 to 7e77204 Compare August 21, 2024 17:17

Raalsky added 5 commits August 21, 2024 19:18

Bump neptune-api version

7a52732

Small refactor

724215a

Small refactor 2

52a2968

Refactors

d65985c

Exceptions and its support

5fdc1e0

Raalsky marked this pull request as ready for review August 21, 2024 21:09

Raalsky requested a review from normandy7 August 21, 2024 21:09

Raalsky commented Aug 21, 2024

View reviewed changes

normandy7 reviewed Aug 22, 2024

View reviewed changes

Raalsky and others added 10 commits August 22, 2024 14:08

Update src/neptune_scale/exceptions.py

9869c68

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

76ef424

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

c9fc83c

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

18ed414

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

d2e1a4b

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/__init__.py

3326007

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

23133ce

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

ef424bf

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/exceptions.py

f3452b1

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

Update src/neptune_scale/__init__.py

4e4a009

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

kgodlewski reviewed Aug 22, 2024

View reviewed changes

Raalsky added 2 commits August 22, 2024 14:55

Code review

8ef15fb

SyncThread renamed

b7a837f

kgodlewski approved these changes Aug 22, 2024

View reviewed changes

kgodlewski merged commit bdf1251 into main Aug 22, 2024
4 checks passed

kgodlewski deleted the rj/requests-tracking branch August 22, 2024 13:08

		)


		def should_print_message(last_message_printed: Optional[float]) -> bool:

chore: Request status tracking #18

chore: Request status tracking #18

Uh oh!

Conversation

Raalsky commented Aug 20, 2024

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!

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kgodlewski Aug 22, 2024 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kgodlewski Aug 22, 2024 •

edited

Loading