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

fix: config schema #526

Merged
merged 4 commits into from
Apr 23, 2024
Merged

fix: config schema #526

merged 4 commits into from
Apr 23, 2024

Conversation

msagi
Copy link
Contributor

@msagi msagi commented Apr 13, 2024

Fix for #509.

  • Updated the schema which did not seem to be correct as per the schema spec ("oneOf" should go under "properties" not the other way around).
  • Moved config file validation logic into the config file module for improved encapsulation
  • Added a few positive and negative test scenarios for config files

@JamieSlome: should GitProxy always validate the config file when starting? It seems it only runs the validation if -v/--validate is present.

Copy link

netlify bot commented Apr 13, 2024

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit cf6aa75
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/662770d0da522600080aab91

Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.69%. Comparing base (71f84e5) to head (cf6aa75).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #526      +/-   ##
==========================================
+ Coverage   64.23%   64.69%   +0.45%     
==========================================
  Files          40       40              
  Lines        1191     1198       +7     
==========================================
+ Hits          765      775      +10     
+ Misses        426      423       -3     

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

@JamieSlome
Copy link
Member

@msagi - I think we should make it such that we validate the config prior to starting the service.

@JamieSlome
Copy link
Member

@msagi - can we create a fixtures folder in the test folder to store all of the mock data?

Copy link
Member

@JamieSlome JamieSlome left a comment

Choose a reason for hiding this comment

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

LGTM! 🍰

Would like to get @coopernetes eyes on this as he pulled together this initial process 👍

@JamieSlome JamieSlome merged commit 33147d4 into finos:main Apr 23, 2024
12 checks passed
Psingle20 pushed a commit to Psingle20/git-proxy that referenced this pull request Nov 27, 2024
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.

3 participants