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

Adds support for clouds.json to Pub Source [SPSTRAT-333] #560

Merged

Conversation

jajreidy
Copy link
Contributor

@jajreidy jajreidy commented Aug 8, 2024

Adds support for clouds.json to Pub source.

The new implementation will try pulling images.json from a Pub task. But if not found then it will try to pull clouds.json from a Pub task.

@jajreidy
Copy link
Contributor Author

jajreidy commented Aug 8, 2024

@rohanpm, @rbikar please review

params = {"format": "raw"}
url = os.path.join(self._url, endpoint, str(task_id), url_ending)

LOG.info("Requesting Pub service for %s of task: %s", url_ending, str(task_id))
LOG.info("Requesting Pub service for JSON file from task: %s", str(task_id))
Copy link
Member

Choose a reason for hiding this comment

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

If you're changing this anyway, might I suggest a log message like "Fetching AMI details from Pub task: %s" makes more sense.

Pub tasks can have JSON files attached for other reasons, so a log message of "Requesting Pub service for JSON file" is a bit vague.

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 makes sense.

src/pushsource/_impl/model/ami.py Show resolved Hide resolved
@jajreidy jajreidy force-pushed the pubsource-cloud-support branch 2 times, most recently from 8c879f9 to 0fc2b7c Compare August 9, 2024 12:13
with Source.get("pub:%s" % pub_url, task_id=task_id) as source:
push_items = [item for item in source]

# there should be exactly one push item
Copy link
Member

Choose a reason for hiding this comment

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

Comment says one but next line says three. Could the false comment be fixed please?

Note, if you want to make the comment more "copy-paste resistant" you could word it like "there should be this many push items". Or even just drop the comment and comparison since the following assertion already implicitly tests the number of push items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All fixed up

@jajreidy jajreidy force-pushed the pubsource-cloud-support branch from 0fc2b7c to e236db7 Compare August 12, 2024 00:44
@rohanpm rohanpm merged commit 7cf0df7 into release-engineering:master Aug 12, 2024
6 checks passed
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