This is a list of all comments for CLOUDSYNC-DEN-16690-1.

Review Summary: No summary

General Comment by Behrouz NematiPour on 15 July 2024, 20:53 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19825

  Merge branch 'DEN-16690-UI-MG-S117' into
  DEN-16839-UI-BN-Whitney-1B1
  c88f546ad09c2d6c0222d8e694a9814e37a424ea
  The follow up CR would be
  http://devapps.diality.us:8060/cru/CLOUDSYNC-DEN-16839-1
  Will be closed.

  Reply by Behrouz NematiPour on 02 August 2024, 14:01
  > RESOLVED


----------------------------------------
File: cloudsync/handlers/ui_cs_request_handler.py
Revision Comment by Behrouz NematiPour on 28 March 2024, 22:50 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19648

  has it been checked if the parameters[0]
  passed/exists/none-zero?

  Reply by Michael Garthwaite on 09 April 2024, 09:47
  > fixed. thanks!

    Reply by Behrouz NematiPour on 09 April 2024, 10:16
    > please push the code to resolve

      Reply by Behrouz NematiPour on 09 April 2024, 13:48
      > RESOLVED.

----------------------------------------
File: cloudsync/utils/helpers.py
Revision Comment by Behrouz NematiPour on 28 March 2024, 22:51 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19649

  does split() always have index 0 ?
  [index out of bounds]

  Reply by Michael Garthwaite on 09 April 2024, 09:47
  > Split will have a value in index 0 as long as the subprocess
  > call is successful. If the subprocess is not successful, the
  > exception will be caught in the message handler and CS will
  > report an error.

    Reply by Behrouz NematiPour on 09 April 2024, 10:18
    > that is the point. what if the du command is not
    > successful?
    > there could be cases that the du call is fine from the
    > python perspective but the result is not a successful or
    > expected result.
    > therefore avoid getting the indexes from array unless you
    > definitely know there is the index.

      Reply by Michael Garthwaite on 09 April 2024, 11:06
      > added exception handling for when the subprocess call
      > fails. thanks

        Reply by Behrouz NematiPour on 09 April 2024, 13:53
        > a catchall exception is not a good idea, please
        > separate the split() and [0], and first check the index
        > exists, the use it.
        > *it is good to keep it if the process fails, but
        > incorrect/unexpected result is not being caught.*

          Reply by Michael Garthwaite on 09 April 2024, 14:12
          > The exception handling was redundant since the
          > message handler will catch it in
          > ui_cs_request_handler.py and log it as
          > CS_LOG_ROTATION_ERROR.
          > 
          > Added check for split() length and if it fails, it'll
          > write to the gutils logger that the subprocess call
          > failed in the chance that subprocess.check_output
          > succeeds but doesn't not contain any data.

          Reply by Behrouz NematiPour on 09 April 2024, 15:50
          > It is fine for now, but as a comment, the part that
          > calls the du command and checks the length and gets
          > the size value should be a function.
          > Please consider for later improvements.
          > 
          > RESOLVED.

Revision Comment by Behrouz NematiPour on 28 March 2024, 22:52 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19650

  does python have functionality/library to check for folder/file
  management/information to be used instead of the shell command
  by a subprocess?

  Reply by Michael Garthwaite on 09 April 2024, 09:43
  > Not that i found that wasnt third party. shutil helps us find
  > the sd card size, but it cant do directory sizes

    Reply by Behrouz NematiPour on 09 April 2024, 13:49
    > RESOLVED

Revision Comment by Behrouz NematiPour on 28 March 2024, 22:55 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19651

  where checked if the sd_used_bytes is zero or not?
  [Divide by zero]

  Reply by Michael Garthwaite on 09 April 2024, 09:45
  > fixed. thanks!

    Reply by Behrouz NematiPour on 09 April 2024, 10:15
    > Please push the fix so it can be resolved.

      Reply by Michael Garthwaite on 09 April 2024, 14:00
      > line 507 has the reassignment if we find that
      > sd_used_bytes is 0

        Reply by Behrouz NematiPour on 09 April 2024, 14:03
        > RESOLVED

Revision Comment by Behrouz NematiPour on 28 March 2024, 22:55 
https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1#c19652

  same here for split?
  [index out of bounds]

  Reply by Michael Garthwaite on 09 April 2024, 09:45
  > Split will have a value in index 0 as long as the subprocess
  > call is successful. If the subprocess is not successful, the
  > exception will be caught in the message handler and CS will
  > report an error.

    Reply by Michael Garthwaite on 09 April 2024, 11:06
    > added exception handling for when the subprocess call
    > fails. thanks

      Reply by Behrouz NematiPour on 09 April 2024, 15:48
      > It is fine for now, but as a comment, the part that calls
      > the du command and checks the length and gets the size
      > value should be a function.
      > Please consider for later improvements.
      > 
      > RESILVED.

        Reply by Dara Navaei on 05 August 2024, 16:06
        > RESOLVED on behalf of [~bnematipour]


---

ID: CLOUDSYNC-DEN-16690-1 https://devapps.diality.us/cru/CLOUDSYNC-DEN-16690-1

Title: CLOUDSYNC-DEN-16690_UI MG S117
Statement of Objectives:


State: Closed

Summary: 

Author: Michael Garthwaite
Moderator: Michael Garthwaite
Reviewers: (4 active, 2 completed*)
     Dara Navaei   (*)
     Behrouz NematiPour   (*)
     Sean Nash
     Tiffany Mejia
     Vinayakam Mani
     msuleiman