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

Enhanced manifest refresh #803

Merged
merged 2 commits into from
Nov 1, 2019

Conversation

vijay8451
Copy link
Member

This PR contains:

  • Refresh-manifest : Remove all subscriptions from Manifest and add only required ones.
  • Test result: /satellite6-manifest-downloader/25

@vijay8451
Copy link
Member Author

vijay8451 commented Oct 9, 2019

added 'DND' as few changes are pending at Job end and @san7ket is working on that now.

automation_tools/__init__.py Outdated Show resolved Hide resolved
Comment on lines +3194 to +3197
command = ('curl -sk -X DELETE -H "Authorization:Basic {0}"'
' "{1}/subscription/consumers/{2}/entitlements"').format(
base64string.decode('utf-8'), url, consumer)
Copy link
Member

@rplevka rplevka Oct 9, 2019

Choose a reason for hiding this comment

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

why not use python requests module for this?
it allows you to parse the response natively

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, i realized. I'm just thinking whether it wouldn't be better to use requests straight away

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea and would handle here #807 if that is fine with you.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rplevka ping :)

@vijay8451 vijay8451 force-pushed the added-full-function branch from d5c15b2 to c7c05ab Compare October 9, 2019 09:33
@vijay8451 vijay8451 force-pushed the added-full-function branch from c7c05ab to e445e67 Compare October 10, 2019 10:00
Copy link
Contributor

@san7ket san7ket left a comment

Choose a reason for hiding this comment

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

You have my ACK, though pending @rplevka comments.

The sequence of the merge should be, If we miss any Job fails

  1. MR in gitlab
  2. Automation Tools PR Enhanced manifest refresh #803
  3. robottelo-ci PR Manifest downloader jobs moves away from environment variables robottelo-ci#1633

Copy link
Contributor

@ntkathole ntkathole 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. ACK

@vijay8451
Copy link
Member Author

Can we merge this and depended PR if no pending on these ?

@san7ket
Copy link
Contributor

san7ket commented Oct 25, 2019

@vijay8451 MR is merged, pending on SatelliteQE/robottelo-ci#1633 I will try to get some review today. If it still pending untill EOD , I will merge and do a test run.

@ntkathole ntkathole merged commit 38e6114 into SatelliteQE:master Nov 1, 2019
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.

4 participants