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

Remove old processed files #86

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Conversation

ILB-96
Copy link
Contributor

@ILB-96 ILB-96 commented Jul 30, 2023

Pull Request Title

Improved Data handling

Related Issue

issue #74

Description

  1. Json files in the Processed folder would stack on top of each other if not deleted manually but now they are removed automatically.
  2. If you had 1-2 files the slider would cause an error because you can't have a slide with 1 item and the default value of the sliders was 2.

Type

  • Bug Fix
  • Feature Enhancement
  • Documentation Update
  • Code Refactoring
  • Other (please specify):

Proposed Changes

  • added files remover function
  • changed the slider default value to 0
  • added a condition to show a slider only if you have more than one file in the folder.

Screenshots / Code Snippets (if applicable)

How to Test

  1. python run_first.py would remove the existing files
  2. streamlite run streamlite_app.py would work even if you have only 1 resume/job description json file

Checklist

  • The code compiles successfully without any errors or warnings
  • The changes have been tested and verified
  • The documentation has been updated (if applicable)
  • The changes follow the project's coding guidelines and best practices
  • The commit messages are descriptive and follow the project's guidelines
  • All tests (if applicable) pass successfully
  • This pull request has been linked to the related issue (if applicable)

Additional Information

@ILB-96 ILB-96 force-pushed the remove_old_processed_files branch from 9a69d7d to cd33156 Compare July 30, 2023 18:12
@srbhr
Copy link
Owner

srbhr commented Jul 31, 2023

Thanks @Hazannovich for this PR. I will test it and then merge it.

Thanks for the PR.

run_first.py Outdated

logging.info('Started to read from Data/Resumes')
try:
# Check if there are resumes present or not.
# If present then parse it.
remove_old_files("Data/Processed/Resumes")

Choose a reason for hiding this comment

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

It would be great if we could define a macro with this string path at the global scope.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah you can add it there as well. @sanketkhajuria

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sanketkhajuria is that fine?

@ILB-96 ILB-96 force-pushed the remove_old_processed_files branch from cd33156 to 931ae89 Compare August 6, 2023 14:21
Copy link
Owner

@srbhr srbhr left a comment

Choose a reason for hiding this comment

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

@Hazannovich Thanks for this mate! 💙💚
This was a much needed feature, I'm approving it and merging it as well.

@srbhr srbhr merged commit 4d2598c into srbhr:main Aug 7, 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.

None yet

3 participants