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

actually load contact info before sending a task message. #4518

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Nov 16, 2021

This is a small change with no associated Issue - description of issue here:

@dpmatthews noticed that a remote platform was not picking up custom settings for task messaging. It appears that these settings aren't loaded from the contact file before they are set for the task messaging app.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.py and
    conda-environment.yml.
  • Does not need tests (to be manually tested?).
  • Appropriate change log entry included.
  • No documentation update required: Part of a soon to be deprecated branch of Cylc.

@wxtim wxtim force-pushed the actually_load_contact_info_before_sending_a_task_message branch from e8816b6 to 95f183f Compare November 17, 2021 08:33
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Looks good, I haven't yet tested it.

@hjoliver
Copy link
Member

Damn, we need another Cylc 7 release.

@oliver-sanders
Copy link
Member

Damn, we need another Cylc 7 release.

We could do with putting this in it too:

#4516

@wxtim wxtim requested review from datamel and removed request for dpmatthews November 19, 2021 15:06
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

I have read the code, checked out the branch and run with a test suite.
I can confirm the branch works as expected and I am happy with the change.

Special thanks to @wxtim for providing me with his test suite and the tips on getting it up and running!

@datamel
Copy link
Contributor

datamel commented Nov 23, 2021

Looks good, I haven't yet tested it.

I will leave the merge for you to finish tests @oliver-sanders. I've done a manual check but you may wish to do one yourself.

@wxtim
Copy link
Member Author

wxtim commented Nov 24, 2021

I have read the code, checked out the branch and run with a test suite. I can confirm the branch works as expected and I am happy with the change.

Special thanks to @wxtim for providing me with his test suite and the tips on getting it up and running!

It was Dave's test suite.

@oliver-sanders oliver-sanders merged commit 7c27b0e into cylc:7.8.x Nov 24, 2021
@wxtim wxtim deleted the actually_load_contact_info_before_sending_a_task_message branch March 22, 2022 17:03
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.

4 participants