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

Feature/SK-1254 | add .fednignore for fedn package #801

Merged
merged 10 commits into from
Feb 5, 2025
Merged

Conversation

Wrede
Copy link
Member

@Wrede Wrede commented Jan 28, 2025

This pull request includes changes to multiple client examples and the fedn CLI to improve the handling of ignore patterns and package creation.

Client Example Updates:

  • Added .ignore files to various client examples to exclude __pycache__, data directories, and specific venv directories. [1] [2] [3] [4] [5] [6] [7] [8]
  • Updated python_env.yaml files to change the environment names to use a dot prefix, ensuring consistency across examples. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

fedn CLI Enhancements:

  • Introduced a new function create_tar_with_ignore in package_cmd.py to create tar archives while respecting ignore patterns specified in .ignore files. This function logs the files being added and handles potential errors gracefully.
  • Modified the create_cmd function to use the new create_tar_with_ignore function, improving the robustness of the package creation process.

@github-actions github-actions bot added the docs label Jan 28, 2025
Copy link
Contributor

@benjaminastrand benjaminastrand left a comment

Choose a reason for hiding this comment

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

Looks very good! I would consider calling the .ignore file something more descriptive like .packageignoreor .fednignore?

@Wrede Wrede changed the title Feature/SK-1254 | add .ignore for fedn package Feature/SK-1254 | add .fednignore for fedn package Jan 29, 2025
@Wrede Wrede requested a review from benjaminastrand January 30, 2025 12:43
@Wrede Wrede removed the docs label Jan 30, 2025
@github-actions github-actions bot added the docs label Jan 30, 2025
Copy link
Contributor

@benjaminastrand benjaminastrand left a comment

Choose a reason for hiding this comment

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

Awesome job man! Great improvement to our software! :)

@Wrede Wrede removed the docs label Feb 3, 2025
Copy link
Contributor

@Hovstadius Hovstadius left a comment

Choose a reason for hiding this comment

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

Running fedn package create -p client creates the .tar and places it inside of the client folder, not the current working directory. Is this an intended update or an unintended bug?

@Wrede
Copy link
Member Author

Wrede commented Feb 4, 2025

Running fedn package create -p client creates the .tar and places it inside of the client folder, not the current working directory. Is this an intended update or an unintended bug?

intended.

@FrankJonasmoelle
Copy link
Contributor

Changes work for my example(s)

@Wrede Wrede merged commit 0d14b4c into master Feb 5, 2025
17 checks passed
@Wrede Wrede deleted the feature/SK-1254 branch February 5, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants