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

feat(2654): process secret in config #2656

Closed
wants to merge 21 commits into from
Closed

feat(2654): process secret in config #2656

wants to merge 21 commits into from

Conversation

dekkku
Copy link
Contributor

@dekkku dekkku commented Aug 11, 2024

Summary:
Briefly describe the changes made in this PR.

Issue Reference(s):
Fixes #2654
/claim #2654
Build & Testing:

  • I ran cargo test successfully.
  • I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

  • I have added relevant unit & integration tests.
  • I have updated the documentation accordingly.
  • I have performed a self-review of my code.
  • PR follows the naming convention of <type>(<optional scope>): <title>

@github-actions github-actions bot added the type: feature Brand new functionality, features, pages, workflows, endpoints, etc. label Aug 11, 2024
output,
schema: self.schema,
preset: self.preset,
secret,
Copy link
Contributor

Choose a reason for hiding this comment

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

Secrets need to be used in the LLM. Read my message — #2658

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i raised pull request for this #2660

@@ -23,6 +23,8 @@ pub struct Config<Status = UnResolved> {
#[serde(skip_serializing_if = "Option::is_none")]
pub preset: Option<PresetConfig>,
pub schema: Schema,
#[serde(skip_serializing_if = "Option::is_none")]
pub secret: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to abstract this out using a NewType

Copy link
Contributor Author

@dekkku dekkku Aug 11, 2024

Choose a reason for hiding this comment

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

created new Secret type now

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using String use

struct TemplateString(Mustache);

Implement it's serde so that you can simply plug and play TemplateString at the time of deserialization instead of introducing an additional step.

Copy link

codecov bot commented Aug 11, 2024

Codecov Report

Attention: Patch coverage is 78.04878% with 9 lines in your changes missing coverage. Please review.

Project coverage is 85.91%. Comparing base (71f3352) to head (d27cefa).

Files Patch % Lines
src/cli/generator/config.rs 78.04% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2656      +/-   ##
==========================================
- Coverage   86.48%   85.91%   -0.57%     
==========================================
  Files         252      252              
  Lines       24291    24331      +40     
==========================================
- Hits        21009    20905     -104     
- Misses       3282     3426     +144     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dekkku dekkku requested a review from tusharmath August 11, 2024 10:52
@tusharmath tusharmath marked this pull request as draft August 11, 2024 11:32
@tusharmath
Copy link
Contributor

Moving to draft to reduce noise and improve CI efficiency. Once you are ready just mark it as "ready to review". Feel free to give a shoutout on the #contributors channel on Discord if you want immediate attention.

@dekkku dekkku changed the title feat: process secret in config feat(2654): process secret in config Aug 11, 2024
@dekkku dekkku closed this Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Bounty claim type: feature Brand new functionality, features, pages, workflows, endpoints, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add secret to generator::config
2 participants