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

🐛Correctly handle file type of on-demand clusters #5101

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Nov 28, 2023

What do these changes do?

This PR fixes the issue where using on-demand clusters would not properly use S3 file types but instead were locked on presigned links.
This had the consequence of limiting uploads to max 5Gib, but is also the sad fact that we still do not properly fixed it by using the public API from the dask-sidecars.

Related issue/s

How to test

Use a python runner and run the following script:

import os


def generate_large_file(file_path, size_gb):
    with open(file_path, "wb") as f:
        # Generate random data
        chunk_size = 1024 * 1024  # 1 MB chunks
        for _ in range(size_gb * 1024):
            data = os.urandom(chunk_size)
            f.write(data)


def create_file():
    # Example usage:
    desired_file_path = "/outputs/large_file.txt"
    desired_size_gb = 6  # Adjust this to your desired size in gigabytes

    generate_large_file(desired_file_path, desired_size_gb)

    print(f"Large file created: {desired_file_path}")


if __name__ == "__main__":
    create_file()

Dev Checklist

DevOps Checklist

  • hotfix to sim4life.io

@sanderegg sanderegg added the a:director-v2 issue related with the director-v2 service label Nov 28, 2023
@sanderegg sanderegg added this to the Kobayashi Maru milestone Nov 28, 2023
@sanderegg sanderegg self-assigned this Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #5101 (a6a9511) into master (00c54cd) will decrease coverage by 0.6%.
Report is 1 commits behind head on master.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #5101     +/-   ##
========================================
- Coverage    70.6%   70.0%   -0.6%     
========================================
  Files         559     563      +4     
  Lines       27394   28596   +1202     
  Branches      207     198      -9     
========================================
+ Hits        19344   20029    +685     
- Misses       8000    8517    +517     
  Partials       50      50             
Flag Coverage Δ
integrationtests 64.8% <100.0%> (-0.1%) ⬇️
unittests 86.3% <100.0%> (+20.9%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...e_service_director_v2/modules/dask_clients_pool.py 93.4% <100.0%> (+5.2%) ⬆️
.../storage/src/simcore_service_storage/exceptions.py 100.0% <100.0%> (ø)
...rage/src/simcore_service_storage/handlers_files.py 99.1% <100.0%> (ø)
...rage/src/simcore_service_storage/simcore_s3_dsm.py 94.3% <100.0%> (ø)

... and 207 files with indirect coverage changes

@sanderegg sanderegg enabled auto-merge (squash) November 28, 2023 18:20
@bisgaard-itis
Copy link
Contributor

bisgaard-itis commented Nov 28, 2023

Should this one be referenced here?

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@sanderegg sanderegg force-pushed the director-v2/correctly-handles-clustertype branch from 62e9875 to a6a9511 Compare November 29, 2023 16:13
Copy link

sonarcloud bot commented Nov 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link

codeclimate bot commented Nov 29, 2023

Code Climate has analyzed commit a6a9511 and detected 0 issues on this pull request.

View more on Code Climate.

@sanderegg sanderegg merged commit 482c544 into ITISFoundation:master Nov 29, 2023
56 checks passed
@sanderegg sanderegg deleted the director-v2/correctly-handles-clustertype branch December 4, 2023 08:57
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jan 8, 2024
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:director-v2 issue related with the director-v2 service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants