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

Upload in-memory files using copy stored on disk #1052

Merged
merged 7 commits into from Oct 26, 2022
Merged

Conversation

shnela
Copy link
Contributor

@shnela shnela commented Oct 21, 2022

No description provided.

@github-actions
Copy link

github-actions bot commented Oct 21, 2022

Test Results

    2 files   -      20      2 suites   - 20   1m 41s ⏱️ - 1h 17m 4s
482 tests  -    347  482 ✔️  -    342  0 💤  -     5  0 ±0 
964 runs   - 5 390  964 ✔️  - 5 286  0 💤  - 104  0 ±0 

Results for commit 3006676. ± Comparison against base commit c43e3d1.

♻️ This comment has been updated with latest results.

@@ -29,6 +30,10 @@ class OperationStorage:
def __init__(self, data_path: str):
self._data_path = Path(data_path)

# initialize directories
os.makedirs(self.data_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe with exist_ok=True

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitelly, good catch!

def get_upload_path(attribute_path: List[str], extension: str, upload_path: Path):
now = datetime.now()
tmp_file_name = f"{'_'.join(attribute_path)}-{now.timestamp()}-{now}.{extension}"
tmp_file_name = tmp_file_name.replace(" ", "_").replace(":", ".")
Copy link
Contributor

Choose a reason for hiding this comment

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

What about just proper strftime() instead of these bad looking replace calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've used to copy-paste this code over and over...
Yes, it's better idea ;)

@shnela shnela merged commit 8a06e89 into master Oct 26, 2022
@shnela shnela deleted the jk/upload-through-hd branch October 26, 2022 10:58
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.

None yet

2 participants