Skip to content

Remove environment variables from create_request_file.py - #529

Merged
Naomi Parsons (NParsonsMO) merged 21 commits into
mainfrom
523-remove-environment-variables-from-create_request_filepy
Aug 28, 2026
Merged

Remove environment variables from create_request_file.py#529
Naomi Parsons (NParsonsMO) merged 21 commits into
mainfrom
523-remove-environment-variables-from-create_request_filepy

Conversation

@NParsonsMO

@NParsonsMO Naomi Parsons (NParsonsMO) commented Jul 14, 2026 •
edited by ehogan
Loading

Copy link
Copy Markdown
Collaborator

Closes #523

PR creation checklist for the developer

  • The <issue_number> above ☝️ has been replaced with the issue number.
  • main has been selected as the base branch.
  • The feature branch name follows the format <issue_number>_<short_description_of_feature>.
  • The text of the PR title exactly matches with the text (not including the issue number) of the issue title.
  • Appropriate reviewers have been added to the PR (once it is ready for review).
  • The PR has been assigned to the developer(s).
  • The same labels as on the issue (except for the good first issue label) have been added to the PR.
  • The Climate Model Evaluation Workflow (CMEW) project has been added to the PR.
  • The appropriate milestone has been added to the PR.

Definition of Done for the developer

  • This PR contains only the changes needed to meet all acceptance criteria.
  • The change in this PR follows the requirements in the wiki: Developer Guide (including copyrights).
  • The GitHub Actions workflow checks pass.
  • The tests run locally and pass (Note: the tests are not run by the GitHub Actions workflow, see wiki: Run the tests locally).
  • Updating the Rose metadata (select one of the following):
    • Rose metadata related to the change has been added or updated.
    • The change does not require Rose metadata to be added or updated.
  • Rendering the Rose metadata (select one of the following):
    • The Rose GUI shows the change as expected.
    • The change in this PR does not affect the Rose GUI.
  • Updating the tests (select one of the following):
    • Tests related to the change have been added or updated.
    • The change does not require tests to be added or updated.
  • Updating the user documentation (i.e. everything in the doc directory, including the Quick Start section; select one of the following):
    • The user documentation related to the change has been updated appropriately.
    • The change in this PR does not require the user documentation to be updated.
  • Rendering the user documentation (wiki: Build the documentation locally provides instructions; select one of the following):
    • The HTML pages show the change as expected.
    • The change in this PR does not affect the HTML pages.
  • Updating the API documentation (e.g. docstrings in Python modules; select one of the following):
    • The API documentation related to the change has been updated appropriately.
    • The change in this PR does not affect the API documentation.

Copyright and IPR:

GitHub handles
I confirm that all code is my own and that my contributions are not subject to copyright or license restrictions. NParsonsMO
I confirm I have not knowingly violated intellectual property rights (IPR) and have taken sensible measures to prevent doing so, including appropriate attribution for usage of Generative AI. I confirm that this work is my own, and I understand that it is my responsibility to ensure I am not violating others’ IPR. This includes taking reasonable steps to ensure that all tools used while creating this contribution did not infringe IPR. NParsonsMO

PR creation checklist for the reviewer

  • The <issue_number> above ☝️ has been replaced with the issue number.
  • main has been selected as the base branch.
  • The feature branch name follows the format <issue_number>_<short_description_of_feature>.
  • The text of the PR title exactly matches with the text (not including the issue number) of the issue title.
  • Appropriate reviewers have been added to the PR (once it is ready for review).
  • The PR has been assigned to the developer(s).
  • The same labels as on the issue (except for the good first issue label) have been added to the PR.
  • The Climate Model Evaluation Workflow (CMEW) project has been added to the PR.
  • The appropriate milestone has been added to the PR.

Definition of Done for the reviewer

  • This PR contains only the changes needed to meet all acceptance criteria.
  • The change in this PR follows the requirements in the wiki: Developer Guide (including copyrights).
  • All developers of this PR have added their names to the Copyright and IPR statements above.
  • The GitHub Actions workflow checks pass.
  • The tests run locally and pass (Note: the tests are not run by the GitHub Actions workflow, see wiki: Run the tests locally).
  • Updating the Rose metadata (select one of the following):
    • Rose metadata related to the change has been added or updated.
    • The change does not require Rose metadata to be added or updated.
  • Rendering the Rose metadata (select one of the following):
    • The Rose GUI shows the change as expected.
    • The change in this PR does not affect the Rose GUI.
  • Updating the tests (select one of the following):
    • Tests related to the change have been added or updated.
    • The change does not require tests to be added or updated.
  • Updating the user documentation (i.e. everything in the doc directory, including the Quick Start section; select one of the following):
    • The user documentation related to the change has been updated appropriately.
    • The change in this PR does not require the user documentation to be updated.
  • Rendering the user documentation (wiki: Build the documentation locally provides instructions; select one of the following):
    • The HTML pages show the change as expected.
    • The change in this PR does not affect the HTML pages.
  • Updating the API documentation (e.g. docstrings in Python modules; select one of the following):
    • The API documentation related to the change has been updated appropriately.
    • The change in this PR does not affect the API documentation.

Important

  • Remember to re-check the Definition of Done after making changes in response to a review.
  • The developer merges the PR.
  • Remember to use the format #<pull_request_number>: <pull_request_title> when writing the merge commit message for the pull request, so the pull request number is immediately visible on GitHub, regardless of the length of the pull request title.

@NParsonsMO Naomi Parsons (NParsonsMO) added enhancement New feature or request configure Anything related to configuration labels Jul 14, 2026
@NParsonsMO Naomi Parsons (NParsonsMO) linked an issue Jul 14, 2026 that may be closed by this pull request
Remove environment variables from create_request_file.py #523
Closed
@NParsonsMO Naomi Parsons (NParsonsMO) added quality assurance Anything related to Quality Assurance (QA) technical debt Technical debt in CMEW labels Jul 14, 2026
@NParsonsMO
Naomi Parsons (NParsonsMO) marked this pull request as ready for review July 14, 2026 10:13
@NParsonsMO

Naomi Parsons (NParsonsMO) commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

This changes a lot of the same files as

so it will be much easier (fewer conflicts) to add in the conftest.py changes once CDDS is updated.

@NParsonsMO
Merge branch 'main' into 523-remove-environment-variables-from-create…
9af7871 
…_request_filepy

# Conflicts:
#	CMEW/app/configure_standardise/bin/configure_standardise.sh
#	CMEW/app/configure_standardise/bin/create_request_file.py
#	CMEW/app/configure_standardise/bin/test_create_request_file.py
@NParsonsMO
Missed at merge
7d36736
@NParsonsMO
Missed at merge
a141994
@NParsonsMO
Merge branch 'main' into 523-remove-environment-variables-from-create…
264d77f 
…_request_filepy
@NParsonsMO
removing argument
f9fc36e
@NParsonsMO
Using conftest.py
8e38656
NParsonsMO
Naomi Parsons (NParsonsMO) commented Aug 7, 2026
View reviewed changes
Comment thread CMEW/app/configure_standardise/bin/test_create_request_file.py
Path(__file__).parent.parent / "etc" / "request_defaults.yml"
)
stream_config_path = Path(__file__).parent.parent / "etc" / "streams.yml"
monkeypatch.setattr(create_request_file, "list_streams", fake_list_streams)

@NParsonsMO Naomi Parsons (NParsonsMO) Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Can / should we avoid monkeypatch here?

@ehogan Emma Hogan (ehogan) Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given what was done in #559, I expect to not have a test for the driver function (i.e. create_request). So happy to leave this for now 👍

@NParsonsMO

Naomi Parsons (NParsonsMO) commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

The request file needs to be made after the variables file so I'm waiting until PR #530 is reviewed.

NParsonsMO
Naomi Parsons (NParsonsMO) commented Aug 10, 2026
View reviewed changes
Comment thread CMEW/app/configure_standardise/rose-app.conf Outdated
@NParsonsMO
Missed removal
551d01b
@NParsonsMO
Missed removal
46fd783
@NParsonsMO
Merge branch 'main' into 523-remove-environment-variables-from-create…
77b0459 
…_request_filepy
@NParsonsMO
Merge branch 'main' into 523-remove-environment-variables-from-create…
07e0ea5 
…_request_filepy

# Conflicts:
#	CMEW/app/configure_standardise/bin/command_line.py
#	CMEW/app/configure_standardise/bin/configure_standardise.sh
#	CMEW/app/configure_standardise/bin/configure_standardise_conftest.py
#	CMEW/app/configure_standardise/rose-app.conf
@NParsonsMO
Copyright
01fc928
ehogan
Emma Hogan (ehogan) requested changes Aug 24, 2026
View reviewed changes

@ehogan Emma Hogan (ehogan) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks Naomi Parsons (@NParsonsMO)! 🥳

Comment thread CMEW/app/configure_standardise/bin/create_request_file.py Outdated
Comment thread CMEW/app/configure_standardise/bin/command_line.py Outdated
Comment thread CMEW/app/configure_standardise/bin/command_line.py Outdated
Comment thread CMEW/app/configure_standardise/bin/command_line.py Outdated
Comment thread CMEW/app/configure_standardise/bin/command_line.py Outdated
Comment thread CMEW/app/configure_standardise/bin/create_request_file.py Outdated
Comment thread CMEW/app/configure_standardise/bin/test_create_request_file.py
Path(__file__).parent.parent / "etc" / "request_defaults.yml"
)
stream_config_path = Path(__file__).parent.parent / "etc" / "streams.yml"
monkeypatch.setattr(create_request_file, "list_streams", fake_list_streams)

@ehogan Emma Hogan (ehogan) Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given what was done in #559, I expect to not have a test for the driver function (i.e. create_request). So happy to leave this for now 👍

ehogan
Emma Hogan (ehogan) approved these changes Aug 28, 2026
View reviewed changes

@ehogan Emma Hogan (ehogan) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks Naomi Parsons (@NParsonsMO) 🎉

I have confirmed that there are no environment variables, nor any occurrences of monkeypatch remaining (other than the one identified) in these files 🎉

CMEW runs successfully for me 🎉

@NParsonsMO
Naomi Parsons (NParsonsMO) merged commit ff616f0 into main Aug 28, 2026
3 checks passed
@NParsonsMO
Naomi Parsons (NParsonsMO) deleted the 523-remove-environment-variables-from-create_request_filepy branch August 28, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ehogan Emma Hogan (ehogan) ehogan approved these changes

Assignees

@NParsonsMO Naomi Parsons (NParsonsMO)

Labels

configure Anything related to configuration enhancement New feature or request quality assurance Anything related to Quality Assurance (QA) technical debt Technical debt in CMEW

Projects

None yet

Milestone

v0.2.0 (multiple model runs)

Development

Successfully merging this pull request may close these issues.

Remove environment variables from create_request_file.py

2 participants

@NParsonsMO @ehogan