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

feat(deb_remote): add sync option parameters #172

Merged

Conversation

joey-grant
Copy link
Contributor

Changeset adds parameter options and tests for:

  • sync_installer
  • sync_sources
  • sync_udebs

@joey-grant
Copy link
Contributor Author

joey-grant commented Jul 26, 2024

This PR addresses #152

Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution.
In order to get this merged, you will need to record the fixtures for the tests. Please take a look at:
https://github.com/pulp/squeezer?tab=readme-ov-file#running-tests-against-a-pulp-in-one-container

Comment on lines 91 to 103
- name: Modify sync_installer on remote
pulp.squeezer.deb_remote:
name: test_deb_remote
sync_installer: true
state: present
register: result
- name: Verify modify sync_installer
assert:
that:
- result.changed == true
- result.remote.sync_installer == true
Copy link
Member

Choose a reason for hiding this comment

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

For the sake of brevity (and testing resources...) can you move all three options into the same two module invocations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdellweg, I combined the tests and added the fixture files. I've squashed the commits and added a fixes #152 to the bottom of the commit messages. Let me know if there is anything out of order here.

@joey-grant joey-grant force-pushed the issue_152_add_deb_remote_sync_parameters branch 2 times, most recently from ccdc2ce to 69dea7f Compare July 27, 2024 14:58
@mdellweg
Copy link
Member

I'm trying to fix the CI issues here: #174

sync_installer:
description:
- Sync installer packages
type: boolean
Copy link
Member

Choose a reason for hiding this comment

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

looks like this needs to be bool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for overlooking that. I've pushed the change. Let me know if there is anything else.

Changeset adds parameter options, tests, and fixtures for:
- sync_installer
- sync_sources
- sync_udebs

fixes pulp#152
@joey-grant joey-grant force-pushed the issue_152_add_deb_remote_sync_parameters branch from 69dea7f to 132efed Compare July 30, 2024 13:25
@mdellweg mdellweg merged commit c1ca0f5 into pulp:develop Jul 30, 2024
7 checks passed
@joey-grant joey-grant deleted the issue_152_add_deb_remote_sync_parameters branch July 30, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants