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.

Sign up for GitHub

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

Jump to bottom

Update comments and options in config files #289

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Update comments and options in config files #289

merged 2 commits into from
Feb 6, 2024

Conversation

tofu-rocketry
Copy link
Member

Resolves #200

  • Set AMS as default messaging system.
  • Add AMS hosts (both prod and devel) as options.
  • Emphasise note about the server_cert.
  • Add token as commented out option.
  • Copy log level comments from sender to receiver conf.

@tofu-rocketry tofu-rocketry added this to the 3.4.0 milestone Jan 22, 2024
@tofu-rocketry tofu-rocketry self-assigned this Jan 22, 2024
@tofu-rocketry tofu-rocketry requested a review from a team as a code owner January 22, 2024 15:11
RedProkofiev
RedProkofiev reviewed Jan 23, 2024
View reviewed changes
conf/receiver.cfg Outdated Show resolved Hide resolved
RedProkofiev
RedProkofiev suggested changes Jan 23, 2024
View reviewed changes
Copy link
Contributor

@RedProkofiev RedProkofiev left a comment

Choose a reason for hiding this comment

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

Change the ssm2test parameter in [messaging], destination: from ssm2test to something that more explicitly tells you what to do with the parameter i.e. changeme

Sae126V
Sae126V reviewed Jan 23, 2024
View reviewed changes
Copy link
Member

@Sae126V Sae126V 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 to me, if we decide to provide default values

@tofu-rocketry
Update comments and options in config files
a8624d8 
- Set AMS as default messaging system.
- Add AMS hosts (both prod and devel) as options.
- Emphasise note about the server_cert.
- Add token as commented out option.
- Copy log level comments from sender to receiver conf.
@tofu-rocketry
Improvements following code-review
c0c6ee3 
- Get rid of placeholder receiver destination. People using this option
  should only be accounting repository admins so will know where they
  need to connect to.
- Make the receiver and sender config match a little better.
- Remove port number from sender config as option is generally less
  likely to be used with AMS being more common.
RedProkofiev
RedProkofiev approved these changes Feb 6, 2024
View reviewed changes
Copy link
Contributor

@RedProkofiev RedProkofiev left a comment

Choose a reason for hiding this comment

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

All looks fine to me.

@tofu-rocketry tofu-rocketry merged commit e28662a into apel:dev Feb 6, 2024
8 checks passed
@tofu-rocketry tofu-rocketry deleted the 200-update-config branch February 6, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sae126V Sae126V Sae126V left review comments

@RedProkofiev RedProkofiev RedProkofiev approved these changes

Assignees

@tofu-rocketry tofu-rocketry

Labels
None yet
Milestone
3.4.0
Development

Successfully merging this pull request may close these issues.

Update default configs
3 participants
@tofu-rocketry @RedProkofiev @Sae126V