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

Allow custom name of the manifest when syncing and publishing #115

Merged
merged 1 commit into from
Sep 11, 2018
Merged

Allow custom name of the manifest when syncing and publishing #115

merged 1 commit into from
Sep 11, 2018

Conversation

vdusek
Copy link

@vdusek vdusek commented Sep 6, 2018

When creating a remote user can specify his own name of the manifest.
As a path to it will be used url/manifest. For example:

$ http POST .../remotes/file/ 
    name=rem1 
    url=https://repos.fedorapeople.org/pulp/pulp/demo_repos/test_file_repo/
    manifest=PULP_MANIFEST

Same situation when creating a publisher.
For example:

$ http POST .../publishers/file/ 
    name=publisher1 
    manifest=my_manifest.csv

closes #3912, #3913
https://pulp.plan.io/issues/3912
https://pulp.plan.io/issues/3913

@vdusek
Copy link
Author

vdusek commented Sep 10, 2018

There's a problem in entering manifest param to the publish request. Example:

$ http POST $PUBLISHER_HREF'publish/' repository=$REPO_HREF manifest='my_manifest.csv'

...
{
    "manifest": [
        "Unexpected field"
    ]
}

The cause of the problem is here:

manifest = serializer.validated_data.get('manifest', 'manifest.csv')

Manifest doesn't pass through serializer validation. It's not part of the model. Is there another way how to get data from command line in the viewset?

@vdusek vdusek changed the title Allow custom name of the manifest Allow custom name of the manifest when syncing and publishing Sep 10, 2018
@vdusek
Copy link
Author

vdusek commented Sep 10, 2018

Update: I didn't notice that there is a suggested solution in issues/3913. So name of the manifest is passed as a field of the publisher.

@daviddavis
Copy link
Contributor

daviddavis commented Sep 10, 2018

I guess the two options regarding publish:

  1. Store a manifest field on the FilePublisher model
  2. Add a manifest parameter to the publish endpoint (e.g. http POST ':8000'$PUBLISHER_HREF'publish/' repository=$REPO_HREF manifest=PULP_MANIFEST)

cc @jortel and @dkliban — any thoughts?

@dkliban
Copy link
Member

dkliban commented Sep 10, 2018

Storing a field on the FilePublisher seems better to me.

Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

Code looks good to me. I think you'll need to update the sync smash tests though.

@jortel
Copy link
Contributor

jortel commented Sep 10, 2018

👍 adding a manifest field to FilePublisher.

@vdusek
Copy link
Author

vdusek commented Sep 10, 2018

Cool, thanks, so I'll update the tests and README.rst.

When creating a remote user can specify his own name of the manifest.
As a path to it will be used url/manifest. For example:
$ http POST .../remotes/file/ name=rem1 url=https://repos.fedorapeople.org/pulp/pulp/demo_repos/test_file_repo/
manifest=PULP_MANIFEST

Same situation when creating a publisher.
For example:
$ http POST .../publishers/file/ name=publisher1 manifest=my_manifest.csv

closes #3912, #3913
https://pulp.plan.io/issues/3912
https://pulp.plan.io/issues/3913
Copy link
Contributor

@jortel jortel left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @vdusek.

@daviddavis daviddavis merged commit abd47ce into pulp:master Sep 11, 2018
@vdusek vdusek deleted the story_3912 branch September 11, 2018 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants