Skip to content

Uploading files could raise an error #193

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

Closed
tobenary opened this issue Dec 26, 2023 · 8 comments
Closed

Uploading files could raise an error #193

tobenary opened this issue Dec 26, 2023 · 8 comments
Labels
bug Something isn't working fixed Fixed in last version

Comments

@tobenary
Copy link

tobenary commented Dec 26, 2023

Describe the bug

I'm trying to upload files using the with open method and upload_stream.
When I'm using filepath as str, everything is working as expected, and the file is uploaded.
When using the "with open" as per the docs, I'm getting encoding errors.
ERROR - 'charmap' codec can't decode byte 0x9d in position 30: character maps to

file_path I used:
"C:\nextconnect\00592295\parsed\12_26_2023_12_06PM_Logs (2)_parsed.7z"

Steps/Code to Reproduce

Not working: (both 'rb' and w/o 'rb', plus, I also tried using encoding='utf-8')

with open(seven_z_upload_file, 'rb') as f:
    file_path = file_path_in_str
    nextcloud_file_object = nc.files.upload_stream(file_path, seven_z_upload_file)

logger.info(f"Uploaded {nextcloud_file_object.full_path}")

Working:

file_path = file_path_in_str
nextcloud_file_object = nc.files.upload_stream(file_path, seven_z_upload_file)

logger.info(f"Uploaded {nextcloud_file_object.full_path}")

Expected Results

Upload files when using the with open, as well as w/o it.
@bigcat88 , as per your example in https://stackoverflow.com/a/77242950/6575170 , it is possible.

Actual Results

Getting error codes like 'charmap' codec can't decode byte 0x9d

Setup configuration

nc_py = 0.71

@bigcat88
Copy link
Member

with open(seven_z_upload_file, 'rb') as f:
file_path = file_path_in_str
nextcloud_file_object = nc.files.upload_stream(file_path, seven_z_upload_file)

logger.info(f"Uploaded {nextcloud_file_object.full_path}")

are you sure that this code should work?

try replacing seven_z_upload_file inside open with f

with open(seven_z_upload_file, 'rb') as f:
    file_path = file_path_in_str
    nextcloud_file_object = nc.files.upload_stream(file_path, f)

@bigcat88
Copy link
Member

or it is again an url encoding problem, and I will fix it tomorrow...

@bigcat88 bigcat88 added the bug Something isn't working label Dec 26, 2023
@tobenary
Copy link
Author

I rechecked it.
it seems this one is working as expected.

            with open(seven_z_upload_file, 'rb') as f:
                file_path = main_nc_folder + ticket_id + '/' + os.path.split(seven_z_upload_file)[1]
                nextcloud_file_object = nc.files.upload_stream(file_path, f)

bigcat88 added a commit that referenced this issue Dec 27, 2023
Ref #193

The same as in #192 but for all other DAV methods.
Tests were expanded to catch this case.

Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
@bigcat88
Copy link
Member

I finished fixing bugs with url encodings, please test the last dev version from repo.

pip install nc_py_api@git+https://github.com/cloud-py-api/nc_py_api

If all is ok, new release will be published in 18 hours.

@tobenary
Copy link
Author

tobenary commented Dec 27, 2023

I tested this code, working as expected:
note - I used this filename:
C:\nextconnect\00567165\Support_tool_analysis#00567165 (2) (3)@#$%& .7z

I'd like to understand also if my code is ok :)
flow:

  • logging to nc.
  • checking if I'm getting a list of files from root, if not, prob. a user error.
  • make a folder named XYZ.
  • if failed, and 405, the folder exists, continue.
  • share that folder with a group.
  • upload a file to that folder.
  • share this file with the group.
  • get the internal link so I'll be able to send it over for downloading it directly.
import os
from nc_py_api import NextcloudException, ShareType, FilePermissions, Nextcloud as nca
import logging


logger = logging.getLogger(__name__)

logging.basicConfig(level=logging.INFO,
                     format="%(asctime)s [%(levelname)s|%(name)s|%(funcName)s] %(message)s",
                     handlers=[logging.FileHandler(r'c:\nextconnect\debug.log', 'a', 'utf-8'), logging.StreamHandler()],
                     encoding='utf-8')

def nc_login(nc):
    try:
        nc.files.listdir('/', 2)
        logger.info(f"Login to NC was Successful")
        return True

    except NextcloudException as e:
        logger.error(f"Login to NC is NOT Successful - {e.status_code}:{e.reason}")
        return False



def create_nc_folder_and_upload_file(ticket_id, seven_z_upload_file):
    nc = nca(nc_auth_user=os.getenv('NEXT_CLOUD_USERNAME'),
             nc_auth_pass=os.getenv('NEXT_CLOUD_PASSWORD'),
             nextcloud_url=os.getenv('NEXT_CLOUD_HOSTNAME')
             )
    success_login_nc = nc_login(nc)
    main_nc_folder = '/SFDC_CASES/SFDC#'
    if success_login_nc:
        try:
            nextcloud_file_object = nc.files.mkdir(main_nc_folder + ticket_id)
            nc_share = nc.files.sharing.create(main_nc_folder + ticket_id,
                                               share_type=ShareType.TYPE_GROUP,
                                               share_with='Support',
                                               permissions=FilePermissions.PERMISSION_READ)
            item_source = str(nc_share.raw_data['item_source'])
        except NextcloudException as res:
            if res.status_code == 405:
                logger.info(f"Folder {ticket_id} already exists. continue")

        try:
            with open(seven_z_upload_file, 'rb') as f:
                file_path = main_nc_folder + ticket_id + '/' + os.path.split(seven_z_upload_file)[1]
                nextcloud_file_object = nc.files.upload_stream(file_path, f)
            # logger.info(f"Uploaded {nextcloud_file_object.full_path}")

        except Exception as e:
            logger.info(f"ERROR - {e}")
        filename_location = (main_nc_folder + ticket_id +
                             '/' + os.path.split(seven_z_upload_file)[1])
        try:
            nc_share = nc.files.sharing.create(nextcloud_file_object,
                                               share_type=ShareType.TYPE_GROUP,
                                               share_with='Support',
                                               permissions=FilePermissions.PERMISSION_READ)

            item_source = str(nc_share.raw_data.get('item_source'))
            if item_source:
                return 'https://nc.sentinelone.com/index.php/f/' + item_source
            if not item_source:
                file_id = str(nextcloud_file_object.info.fileid)
                return 'https://nc.sentinelone.com/index.php/f/' + file_id
        except NextcloudException as res:
            if res.status_code == 401:
                logger.info("folder already shared to group. continue")
                file_id = str(nextcloud_file_object.info.fileid)
                return 'https://nc.sentinelone.com/index.php/f/' + file_id

print(create_nc_folder_and_upload_file('00593445',
                                 r"C:\nextconnect\00567165\Support_tool_analysis#00567165 (2)   ("
                                 r"3)@#$%& .7z"))

@bigcat88
Copy link
Member

The code looks good, but looking at the code brought up two questions.

  1. Is there a need for a separate login function that can be called manually so that you can easily check whether the login was successful or not?

Like: nc.login() or nc.perform_login()

  1. nc_share.raw_data.get("item_source")
    I missed this place, usually now I write “_raw_data” everywhere.
    Apparently I just didn’t fully describe it(class Share) and therefore didn’t convert the variable to Protected. I'll expand and complete "class Share" and rename "nc_share.raw_data" to "nc_share._raw_data" ...

  2. Do we need to define more Nextcloud** exceptions to allow easy exception ignore like:
    "with contextlib.suppress(Nextcloud***):" ?
    Currently we have only: NextcloudExceptionNotModified, NextcloudExceptionNotFound, NextcloudMissingCapabilities
    And using with contextlib.suppress(NextcloudExceptionNotFound) to ignore errors in some cases is much cleaner imho.

  3. If we expand exceptions list, then should we or not rename NextcloudExceptionNotFound to NCExceptionNotFound to make all exceptions shorter..

P.S: "if res.status_code == 401:" -> 401 is Status Unauthorized

@tobenary
Copy link
Author

tobenary commented Dec 28, 2023

  1. I think yes, you always need to perform tasks and you need to check your connection.
  • In my code, I'm doing the obvious, checking the root folder for files. (even in the first installation, I remember 4 files are auto added )
  • If the server is not running or someone changed the password / else, I return False.
  1. Is it possible to reply also with the item_source every NF call? (files+sharing)
  2. I don't know the right answer, sorry :) You're the PM here.
  3. Well, it could be nice, but again, not huge.

P.S, I know :)

@bigcat88
Copy link
Member

2. Is it possible to reply also with the item_source every NF call? (files+sharing)

Adding extra fields to class Share I leaved for 0.8.0 version.

@bigcat88 bigcat88 added the fixed Fixed in last version label Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Fixed in last version
Projects
None yet
Development

No branches or pull requests

2 participants