Skip to content

Conversation

@bcwu
Copy link
Contributor

@bcwu bcwu commented Oct 28, 2022

Description

Update actions.py to reuse the code in main, minus the CLI components.

@bcwu bcwu marked this pull request as ready for review October 28, 2022 20:00
base_dir = dirname(file_name)
_warn_on_ignored_manifest(base_dir)
_warn_if_no_requirements_file(base_dir)
_warn_if_environment_directory(base_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a little odd for API usage to trigger console warnings. We should ask about whether that's a desired behavior.

app_mode = read_manifest_app_mode(manifest_file_name)
kwargs["title"] = title or default_title_from_manifest(manifest_file_name)

if isinstance(connect_server, api.RSConnectServer):
Copy link
Contributor

Choose a reason for hiding this comment

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

This code block occurs repeatedly - let's factor out a helper that returns a dict we can pass to kwargs.update.

Copy link
Collaborator

@isabelizimm isabelizimm left a comment

Choose a reason for hiding this comment

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

popping in here to say I tested this out with vetiver, and it feels really nice! fixes the requirements bugs I had when using python_deploy_fastapi

@bcwu bcwu merged commit 0d367dd into master Nov 7, 2022
@bcwu bcwu deleted the bcwu-update-actions branch November 7, 2022 16:35
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.

4 participants