-
Notifications
You must be signed in to change notification settings - Fork 6
FI-3604 Run evaluate without test kit using tmp dir #674
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #674 +/- ##
=======================================
Coverage 85.00% 85.00%
=======================================
Files 303 303
Lines 13677 13677
Branches 1760 1760
=======================================
Hits 11626 11626
Misses 2043 2043
Partials 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Is it possible to run inferno "globally" without needing a gemfile in the current directory? |
The Gemfile is only needed for this PR, because you have to point at this branch rather than a released gem. |
|
||
Dir.chdir(tmpdir) do | ||
Migration.new.run(Logger::FATAL) # Hide migration output for evaluator | ||
evaluate(ig_path, data_path, options) |
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.
Won't data_path
need to be updated to an absolute path prior to changing the working directory?
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.
fixed
return @service_names if @service_names | ||
|
||
fhirpath_health = Faraday.get("#{ENV.fetch('FHIRPATH_URL', nil)}/version").status | ||
@service_names = "hl7_validator_service #{fhirpath_health == 200 ? '' : 'fhirpath'}" |
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.
Why is this logic only needed for the fhirpath service? I would expect it to be present for the validator service as well, or just a plain docker compose up
without the service names.
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.
The validator service doesn't expose its port so it doesn't conflict with spinning it up for the evaluator. On the other hand if we wanted the evaluator to leverage an already running validator service we would have to route it through Nginx or modify test kits to expose the validator port as well.
lib/inferno/config/boot/db.rb
Outdated
@@ -9,7 +9,7 @@ | |||
|
|||
Sequel::Model.plugin :json_serializer | |||
|
|||
config_path = File.expand_path('database.yml', File.join(Dir.pwd, 'config')) | |||
config_path = File.expand_path('../../../../config/database.yml', __dir__) |
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.
This change prevents users from customizing the database configuration, which is a necessary feature. Also, the config
directory is not included in the inferno_core
gem.
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 reverted this line and added a new database config specifically for the evaluator that uses sqlite in memory.
Summary
Run
inferno execute
without being in a test kit by automatically spinning up the required services and putting Docker mounts in a temporary directory. After a release with this PR a user should be able to do:Testing Guidance
Example Gemfile
Example command: