Skip to content

Make config required arg in ert_api_parser#13685

Draft
jaakson wants to merge 1 commit into
equinor:mainfrom
jaakson:13602-ert-api-zero-arguments-bug
Draft

Make config required arg in ert_api_parser#13685
jaakson wants to merge 1 commit into
equinor:mainfrom
jaakson:13602-ert-api-zero-arguments-bug

Conversation

@jaakson
Copy link
Copy Markdown

@jaakson jaakson commented Jun 2, 2026

The ert api subparser was being passed to an add_parser_options function that added config as an optional argument when it should be required.

Issue
Resolves #13602

Approach

  • Dropped config from add_parser_options

  • Moved the optional config arg addition where it was needed

  • Added required config to ert_api_parser the way it's done for other subparsers

  • PR title captures the intent of the changes, and is fitting for release notes.

  • Added appropriate release note label

  • Commit history is consistent and clean, in line with the contribution guidelines.

  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.91%. Comparing base (e768c62) to head (82d59a3).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
src/ert/services/_storage_main.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13685      +/-   ##
==========================================
+ Coverage   89.38%   89.91%   +0.52%     
==========================================
  Files         469      468       -1     
  Lines       33391    33403      +12     
==========================================
+ Hits        29847    30034     +187     
+ Misses       3544     3369     -175     
Flag Coverage Δ
cli-tests 35.79% <50.00%> (+0.03%) ⬆️
fuzz 43.38% <50.00%> (-0.05%) ⬇️
gui-tests 59.28% <50.00%> (-0.06%) ⬇️
performance-and-unit-tests 77.82% <50.00%> (-0.03%) ⬇️
test 45.90% <0.00%> (+0.92%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ert/__main__.py 85.94% <100.00%> (+0.04%) ⬆️
src/ert/services/_storage_main.py 43.60% <0.00%> (-0.59%) ⬇️

... and 16 files with indirect coverage changes

@andreas-el
Copy link
Copy Markdown
Contributor

You can consider adding a test in test_cli.py

Possibly pick up output using;

ert_process = Popen(<command>, stdout=PIPE, stderr=PIPE)
stdout, stderr = ert_process.communicate()

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 2, 2026

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing jaakson:13602-ert-api-zero-arguments-bug (25206e7) with main (e7fc16b)

Open in CodSpeed

Move optional config argument from add_parser_options to parse_args
Add test for API CLI error when config argument is missing
@jaakson jaakson force-pushed the 13602-ert-api-zero-arguments-bug branch from 25206e7 to 82d59a3 Compare June 3, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ert api accepts zero arguments but fails with misleading error message if no config is given

3 participants