feat: baml-cli support for Boundary Cloud #1046
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to b6f6437 in 1 minute and 4 seconds
More details
- Looked at
3637lines of code in46files - Skipped
4files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. engine/cli/src/propelauth.rs:132
- Draft comment:
Log the error whenopen::thatfails to provide more context for debugging. - Reason this comment was not posted:
Confidence changes required:50%
Therequest_authorization_codefunction inpropelauth.rsdoes not handle the case where theopen::thatfunction fails. It should log the error for better debugging.
2. engine/cli/src/deploy.rs:283
- Draft comment:
Consider reusing a singlereqwest::Clientinstance for multiple requests instead of creating a new one each time. This can improve performance by reusing connections. - Reason this comment was not posted:
Marked as duplicate.
3. engine/cli/src/auth.rs:68
- Draft comment:
Handle the error case forPersistedTokenData::read_from_storage()to provide a user-friendly message or guidance on resolving the issue. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is suggesting a code improvement that is actionable and clear. It is not asking for confirmation or making speculative statements. The suggestion is directly related to the code change in the diff.
The comment could be seen as unnecessary if the existing error handling is deemed sufficient by the project's standards. However, providing user-friendly error messages is generally a good practice.
Even if the current error handling is technically sufficient, enhancing user experience with clearer error messages is beneficial and aligns with good coding practices.
The comment is valid as it suggests an actionable improvement to the code by enhancing error handling for better user experience.
Workflow ID: wflow_3aErTaG8P0HfxHvn
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| &self, | ||
| req: GetOrCreateProjectRequest, | ||
| ) -> Result<GetOrCreateProjectResponse> { | ||
| let resp = reqwest::Client::new() |
There was a problem hiding this comment.
Consider reusing a single reqwest::Client instance for multiple requests instead of creating a new one each time. This can improve performance by reusing connections.
There was a problem hiding this comment.
don't we have a static builder? We should use that one i think cause it has some settings that make it configure correctly.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 60a2e39 in 35 seconds
More details
- Looked at
65lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_zJnCSVGlgzO3c5xZ
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
engine/cli/src/deploy.rs
Outdated
| println!(" https://dashboard.boundaryml.com/projects/{project_id}/api-keys"); | ||
| println!(); | ||
| println!("3. Call your functions!"); | ||
| println!(" https://dashboard.boundaryml.com/projects/{project_id}/api-keys"); |
There was a problem hiding this comment.
The URL for calling functions is repeated. Consider removing the duplicate line to avoid confusion.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 985cd64 in 25 seconds
More details
- Looked at
31lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. docs/docs.yml:54
- Draft comment:
Inconsistent indentation. Ensure consistent indentation for proper YAML parsing. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_WEUpt7OIlvb4KEx4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b9afff5 in 35 seconds
More details
- Looked at
13lines of code in1files - Skipped
1files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. docs/docs.yml:58
- Draft comment:
The change fromsection: NextJStopage: Nextjsis incorrect. It should remain assectionto maintain consistency with the rest of the structure. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment suggests that the change from 'section' to 'page' is incorrect. However, the context shows that 'page' is used frequently in similar contexts, indicating that the change might be intentional and consistent with other parts of the file. Without strong evidence that 'section' is the correct term here, the comment seems speculative.
I might be missing specific knowledge about the structure of this YAML file and whether 'section' or 'page' is the correct term in this context. The comment could be correct if there's a specific convention being violated.
The frequent use of 'page' in similar contexts suggests that the change might be intentional and correct. Without clear evidence that 'section' is required here, the comment seems speculative.
Delete the comment as it seems speculative and lacks strong evidence that the change is incorrect.
Workflow ID: wflow_a8YfOtB7fNioN7R4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 5193c52 in 42 seconds
More details
- Looked at
211lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. engine/cli/src/deploy.rs:160
- Draft comment:
Theget_or_create_projectfunction does not handle the case where there are multiple projects. This will cause the function to fail if there are multiple projects, which is not the intended behavior according to the comments. Consider implementing an interactive prompt to allow the user to select a project. - Reason this comment was not posted:
Comment was on unchanged code.
2. engine/cli/src/deploy.rs:227
- Draft comment:
Theget_or_create_projectfunction does not handle the case where there are multiple projects. This will cause the function to fail if there are multiple projects, which is not the intended behavior according to the comments. Consider implementing an interactive prompt to allow the user to select a project. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_MwSn06R2Rb3cmd3R
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on dbe6a23 in 25 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. engine/cli/src/deploy.rs:289
- Draft comment:
The use ofshift_removeto remove "generators.baml" frombaml_srcis not explained. If the intention is to remove this specific file, ensure that it is present in theIndexMapbefore callingshift_remove. If not, this could lead to unexpected behavior. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_grq20tmiO3rp8VL2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 40ccf24 in 56 seconds
More details
- Looked at
119lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_XGdM7klFisTXHkrt
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| return Err(resp | ||
| .error_for_status() | ||
| .context("Failed to get or create project") | ||
| .unwrap_err()); |
There was a problem hiding this comment.
Using unwrap_err() here is unnecessary. You can directly use context to provide the error context without unwrapping the error.
| .unwrap_err()); | |
| .context("Failed to get or create project")? |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 220a830 in 26 seconds
More details
- Looked at
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. engine/cli/src/deploy.rs:109
- Draft comment:
Remove the commented-out code// env!("CARGO_PKG_VERSION")for clarity, asenv!("CARGO_PKG_VERSION")is correctly used. - Reason this comment was not posted:
Confidence changes required:50%
The use ofenv!("CARGO_PKG_VERSION")is correct and aligns with the intent to dynamically fetch the package version. However, the commented-out code should be removed for clarity.
Workflow ID: wflow_R6YrNaUVIJLghYGy
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Important
Add BAML CLI support for deploying projects to Boundary Cloud with authentication and deployment functionalities.
baml-cli.get_or_create_projectanddeploy_new_projectindeploy.rs.propelauth.rs.auth,login, anddeployadded tocommands.rs.run_clifunction inlib.rsinitializes and runs the CLI.docs.ymlto include Boundary Cloud documentation.host-your-baml-functions.mdxfor hosting BAML functions on Boundary Cloud.api_client.rsfor API interactions.FutureWithProgresstrait intui.rsfor progress indication.Cargo.tomlfiles to include new dependencies and features.This description was created by
for 220a830. It will automatically update as commits are pushed.