Skip to content
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

Limited upload size #10

Closed
wants to merge 11 commits into from
Closed

Limited upload size #10

wants to merge 11 commits into from

Conversation

SAMAD101
Copy link
Member

addressed in #5

@Mr-Sunglasses Mr-Sunglasses linked an issue Dec 22, 2023 that may be closed by this pull request
@Mr-Sunglasses
Copy link
Member

hey @SAMAD101 test's are failing could you please check.

@SAMAD101
Copy link
Member Author

hey @SAMAD101 test's are failing could you please check.

sure

@Mr-Sunglasses
Copy link
Member

Also @SAMAD101 I have closed your PR #11 Due to the same reason. could you do the PR again

@SAMAD101
Copy link
Member Author

Also @SAMAD101 I have closed your PR #11 Due to the same reason. could you do the PR again

Sure I will

Copy link
Member

@A91y A91y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason of failing test cases is that you forgot to add return statement in

@app.get("/paste/{uuid}")
def post_as_a_text(uuid):
...

@A91y
Copy link
Member

A91y commented Dec 28, 2023

The reason of failing test cases is that you forgot to add return statement in

@app.get("/paste/{uuid}")
def post_as_a_text(uuid):
...

cc @SAMAD101

@A91y
Copy link
Member

A91y commented Dec 28, 2023

The corrected function is:

@app.get("/paste/{uuid}")
def post_as_a_text(uuid):
    path = f"data/{uuid}"
    text = ""
    try:
        with open(path, 'rb') as f:
            text = f.read()
        if sys.getsizeof(text) > MAX_UPLOAD_SIZE:
            raise HTTPException(detail="File size is too large",
                                status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE)
        else:
            return PlainTextResponse(text)
    except Exception as e:
        print(e)
        raise HTTPException(detail="404: The Requested Resource is not found",
                            status_code=status.HTTP_404_NOT_FOUND)

@SAMAD101
Copy link
Member Author

The reason of failing test cases is that you forgot to add return statement in

@app.get("/paste/{uuid}")
def post_as_a_text(uuid):
...

cc @SAMAD101

makes sense

@SAMAD101
Copy link
Member Author

The corrected function is:

@app.get("/paste/{uuid}")
def post_as_a_text(uuid):
    path = f"data/{uuid}"
    text = ""
    try:
        with open(path, 'rb') as f:
            text = f.read()
        if sys.getsizeof(text) > MAX_UPLOAD_SIZE:
            raise HTTPException(detail="File size is too large",
                                status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE)
        else:
            return PlainTextResponse(text)
    except Exception as e:
        print(e)
        raise HTTPException(detail="404: The Requested Resource is not found",
                            status_code=status.HTTP_404_NOT_FOUND)

thanks

@Mr-Sunglasses
Copy link
Member

@SAMAD101 Could you add a test for it [ which fails ].

@Mr-Sunglasses
Copy link
Member

Also Please update this branch to latest main [ head ]

Copy link
Member

@Mr-Sunglasses Mr-Sunglasses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests for it and sync with the main head

@SAMAD101
Copy link
Member Author

SAMAD101 commented Dec 28, 2023

@Mr-Sunglasses the test that were failing, now passes. I think we are good to go
Just resolved some conflicts with main [Head]
The test will fail now (tested locally), coz of the rate limiter. We can go ahead and merge this one.
I will look into writing on another branch under this under this issue #26
wait @A91y already did in #27

SAMAD101 and others added 2 commits December 28, 2023 22:26
@Mr-Sunglasses
Copy link
Member

Why it is failing because of rate limiter :(

@Mr-Sunglasses
Copy link
Member

@SAMAD101 just do one thing close this PR and open a new fresh one, also write tests to check for Limited upload size.

@SAMAD101 SAMAD101 closed this Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add limit to the size of uploaded file.
3 participants