-
Notifications
You must be signed in to change notification settings - Fork 6
Infrahubctl repo Init command #371
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
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
3fb79ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7f37a203.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://atg-20250219-cs60.infrahub-sdk-python.pages.dev |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #371 +/- ##
===========================================
- Coverage 73.91% 73.88% -0.03%
===========================================
Files 92 92
Lines 8536 8555 +19
Branches 1676 1679 +3
===========================================
+ Hits 6309 6321 +12
- Misses 1781 1787 +6
- Partials 446 447 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
|
||
| **Arguments**: | ||
|
|
||
| * `DST`: [required] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear what DST is, or what it is that we are supposed to provide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should DST be optional? If not provided, we could make a directory from the project name that we prompt for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately from what I read copier requires the destination to be known upfront before the prompt appears using the run_copy function. By default copier will use cwd if nothing is passed in. which would require this workflow. We are using package name more for the pyproject.toml package name.
mkdir infrahub-repo`
cd infrahub-repo
infrahubctl repository init
Rather than just infrahubctl repository init infrahub-repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DST now also has help to explain what it is.
|
|
||
| **Options**: | ||
|
|
||
| * `--data PATH` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, what is data in this case? A Path to a file? To what file? To a directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added helpers to the arguments for the docs to be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this options for the CLI command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what --data does allows user to pass in arguments via a file. Unfortunately defining options in two places I.E in the typer CLI and then in Copier prevents the copier prompts from appearing and will only use the CLI argument defaults. The idea of copier is that it remembers what the user selected previously so that repos can be updated in future where as the typer CLI does not.
|
While I can see that this could be helpful for some people I think it would also be nice if we didn't have to add extra dependencies to the ctl. :) Other solutions could potentially be to have some example repos that are easy to clone and use as a starting point. Anyway some observations about this PR:
|
|
I would consider not loading the schema file. The recommendation is that when you manage the schema through the repository that you don't use Some people find it very confusing that schema changes loaded using |
infrahubctl repository init /tmp/testwill create a new repository with some basic structure to get started with Infrahub. It will prompt the user what they would like to include.