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

Add relative_url as a valid config key for export distributor #743

Closed
wants to merge 0 commits into from

Conversation

seandst
Copy link
Contributor

@seandst seandst commented Dec 15, 2015

https://pulp.plan.io/issues/1394
fixes #1394

pulp-admin support is conspicuously absent. There are some decisions to make for that, so I've opened up a new issue related to 1394 to deal with them.

@seandst seandst closed this Dec 15, 2015
@seandst seandst reopened this Dec 15, 2015
@seandst
Copy link
Contributor Author

seandst commented Dec 17, 2015

ok test

1 similar comment
@asmacdo
Copy link
Contributor

asmacdo commented Dec 17, 2015

ok test

@ipanova ipanova self-assigned this Jan 4, 2016
@ipanova
Copy link
Member

ipanova commented Jan 5, 2016

@seandst I think originally you checkouted from 2.7 and made the changes. So either you submitted against wring branch or you were supposed to checkout from master. Please fix that

@@ -109,6 +109,10 @@ def validate_export_config(config):
if not os.path.isabs(value):
msg = _("Value for 'export_dir' must be an absolute path: %s" % value)
return False, msg
if key == constants.RELATIVE_URL_KEYWORD:
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's intentionally left out of this PR.

@ipanova
Copy link
Member

ipanova commented Jan 5, 2016

@seandst also i think you should add the condition check of the situation where both export_dir and relative_url are provided and raise the error that you cannot use them together.

@ipanova ipanova assigned seandst and unassigned ipanova Jan 5, 2016
@beav
Copy link
Contributor

beav commented Jan 5, 2016

Once this is merged, I can test it off of the nightly.

@seandst
Copy link
Contributor Author

seandst commented Jan 5, 2016

@beav it'll be a little while, I think, since this is currently (to use a technical term) broken as crap

I'll keep you updated. :)

@seandst seandst force-pushed the 1394-relative-url-export-config branch 2 times, most recently from f55e15c to d97636d Compare January 7, 2016 02:08
@seandst seandst removed their assignment Jan 7, 2016
@seandst
Copy link
Contributor Author

seandst commented Jan 7, 2016

This has been fixed up and should now work as intended, so it's ready for review.

I also added a fix to #1419, so you can now use the --relative-url option when using the pulp-admin rpm repo export subcommand to alter the location for that export.

You can also use the rest api to make the change before the export call. httpie e.g. http PUT https://localhost/pulp/api/v2/repositories/zoo/distributors/export_distributor/ distributor_config:='{"relative_url": "foo"}'

@seandst
Copy link
Contributor Author

seandst commented Jan 7, 2016

ok test

@ipanova ipanova self-assigned this Jan 7, 2016
@ipanova
Copy link
Member

ipanova commented Jan 8, 2016

@seandst i noticed that during repo creation( via pulp-admin), when you specify --relative url option it affects only yum_dist and not export_dist. Via API it works as supposed.

$ pulp-admin -vv rpm repo create --repo-id myrepo --relative-url different-url
Warning: path should have mode 0700 because it may contain sensitive information: /home/ipanova/.pulp/

2016-01-08 12:31:39,438 - DEBUG - sending POST request to /pulp/api/v2/repositories/
2016-01-08 12:31:39,583 - INFO - POST request to /pulp/api/v2/repositories/ with parameters {"display_name": null, "description": null, "distributors": [{"distributor_id": "yum_distributor", "auto_publish": true, "distributor_config": {"http": false, "relative_url": "different-url", "https": true}, "distributor_type_id": "yum_distributor"}, {"distributor_id": "export_distributor", "auto_publish": false, "distributor_config": {"http": false, "https": true}, "distributor_type_id": "export_distributor"}], "notes": {"_repo-type": "rpm-repo"}, "importer_type_id": "yum_importer", "importer_config": {}, "id": "myrepo"}
2016-01-08 12:31:39,584 - INFO - Response status : 201 

2016-01-08 12:31:39,584 - INFO - Response body :
 {
  "scratchpad": {}, 
  "display_name": null, 
  "description": null, 
  "last_unit_added": null, 
  "notes": {
    "_repo-type": "rpm-repo"
  }, 
  "last_unit_removed": null, 
  "content_unit_counts": {}, 
  "_ns": "repos", 
  "_id": {
    "$oid": "568f9e1b45ef4815db1c2d13"
  }, 
  "id": "myrepo", 
  "_href": "/pulp/api/v2/repositories/myrepo/"
}

Successfully created repository [myrepo]

$ curl -H "Accept: application/json" -X GET -k -u admin:admin 'https://localhost/pulp/api/v2/repositories/myrepo/distributors/export_distributor/'|python -m json.tool
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   368  100   368    0     0   1273      0 --:--:-- --:--:-- --:--:--  1273
{
    "_href": "/pulp/api/v2/repositories/myrepo/distributors/export_distributor/",
    "_id": {
        "$oid": "568f9e1b45ef4815db1c2d16"
    },
    "_ns": "repo_distributors",
    "auto_publish": false,
    "config": {
        "http": false,
        "https": true
    },
    "distributor_type_id": "export_distributor",
    "id": "export_distributor",
    "last_publish": "2016-01-08T11:38:07Z",
    "repo_id": "myrepo",
    "scratchpad": {}
}

@ipanova
Copy link
Member

ipanova commented Jan 8, 2016

@seandst as far as i see you need to add relative_url to the export_dist_config_keys, which defaults to repo-id https://github.com/seandst/pulp_rpm/blob/1394-relative-url-export-config/extensions_admin/pulp_rpm/extensions/admin/repo_create_update.py#L37

@ipanova ipanova assigned seandst and unassigned ipanova Jan 8, 2016
@seandst
Copy link
Contributor Author

seandst commented Jan 8, 2016

@seandst i noticed that during repo creation( via pulp-admin), when you specify --relative url option it affects only yum_dist and not export_dist. Via API it works as supposed.

This is how it should work. Having relative-url affect the export distribute would be a backward-incompabile change. That's what https://pulp.plan.io/issues/1419 is about, so we can talk about it more there.

@seandst seandst added the LGTM label Jan 8, 2016
@seandst
Copy link
Contributor Author

seandst commented Jan 8, 2016

I got a conditional and verbal lgtm from ipanova after an IRC discussion. The condition is that the pulp-admin commit get moved into a second PR, and we'll discuss that bit further and address it in redmine #1419.

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.

None yet

4 participants