-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Support reflex app creation from templates from github #2490
Conversation
798e47d
to
2c62bfe
Compare
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.
Works well for me (though I wasn't able to connect to get the list of templates). Maybe for now we can leave out the interactive init and support initialization with the --template
flag only. I think I they do --template
without any args, that's where we could show all the templates we offer through the CLI.
reflex/reflex.py
Outdated
template = prerequisites.prompt_for_template() | ||
prerequisites.create_config(app_name) | ||
prerequisites.initialize_app_directory(app_name, template) | ||
# Fetch App templates from the backend server |
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.
Let's move all of this to a function in prerequisites.py
to keep this function clean
reflex/utils/prerequisites.py
Outdated
def initialize_app_directory( | ||
app_name: str, | ||
template_name: str = constants.Templates.Kind.BLANK.value, | ||
template_code_dir_name: Optional[str] = None, |
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 think we can do | None
instead of Optional outside of Pydantic use cases
reflex/utils/prerequisites.py
Outdated
except OSError as ose: | ||
console.error(f"Failed to create temp directory for download: {ose}") | ||
raise typer.Exit(1) from ose | ||
# Use httpx GET with redirects to download the zip file |
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.
nit: can we put blank lines before each comment to make the code more readable
reflex/utils/prerequisites.py
Outdated
A valid template name. | ||
""" | ||
if template is not None: | ||
# If user selects a template, it needs to exist |
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.
nit: end comments with periods for consistency
reflex/utils/prerequisites.py
Outdated
# the source code repo name on github | ||
template_name = new_config.app_name | ||
|
||
# Prompt for Reflex App name change |
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 we move this section to its own function? I think this may in the future be useful as its own subcommand reflex init --rename <new_name>
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.
remove this section for now. Let me do that when we add --rename. The rename is slightly differently I suppose. Here, we basically generate a new rxconfig file instead regex replace any strings. I suspect we might have to do string substitution when it a generic app. There might be other attributes added in the template app.
do you know how to achieve that in typer option? From what I experimented, if it's not a boolean flag, it'll expect something set for that flag or no flag at all for None. |
cfa5db2
to
07ea15b
Compare
reflex/utils/prerequisites.py
Outdated
templates = fetch_app_templates() or {} | ||
if template is None: | ||
template = prompt_for_template(templates) | ||
templates: dict[str, Template] = fetch_app_templates() or {} |
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.
nit: fetch_app_templates
always return a dict, no need for or {}
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.
good catch, fixed
When changing page in a stateless app, there is no state to set `is_hydrated: False`
a034a8a
to
d190d64
Compare
reflex/utils/prerequisites.py
Outdated
template_url = templates[template].code_url | ||
else: | ||
# Check if the template is a github repo. | ||
if "github.com" not in template: |
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 it be a more strict check something like it must start with https://github.com/
or something?
reflex/utils/prerequisites.py
Outdated
console.error(f"Template `{template}` not found.") | ||
raise typer.Exit(1) | ||
repo = template.split("github.com/")[1].strip("/") | ||
template_url = f"https://github.com/{repo}/archive/main.zip" |
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.
looks like we expect the template_url to be of https://github.com/owner/repo/
format, do we want to just right strip /
then append main.zip
which also means we can't specify a branch for now. Likely ok
Summary
blank
is shown to user.blank
.Usage:
reflex init --template https://github.com/reflex-dev/reflex-chat
Tests
blank_template
: https://github.com/reflex-dev/blank-template/archive/main.zipchat_app
: https://api.github.com/repos/reflex-dev/reflex-chat/zipball/ee5b472