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.

Sign up for GitHub

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

Jump to bottom

feat: add methods to put and update with files #105

Merged
merged 2 commits into from
Jun 7, 2024
Merged

feat: add methods to put and update with files #105

merged 2 commits into from
Jun 7, 2024

Conversation

skatsaounis
Copy link
Contributor

This PR enables updating MAAS resources with files included.

Acts as an enabler for: PUT /MAAS/api/2.0/scripts/{name}: Update a script

Source: https://maas.io/docs/api

@jujubot
Copy link
Contributor

jujubot commented May 2, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

1 similar comment
@jujubot
Copy link
Contributor

jujubot commented May 2, 2024

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

@skatsaounis
Copy link
Contributor Author

NOTE: Post already accepts files but I kept the existing Put intact so that we will not break existing consumers of the gomaasapi. I could update Put directly if you like it more. Please let me know 🙂

@skatsaounis skatsaounis mentioned this pull request May 3, 2024
Merged
manadart
manadart reviewed May 27, 2024
View reviewed changes
Copy link
Member

@manadart manadart 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 proposing this change.

Can we get some tests and steps to verify correctness added?

@skatsaounis
Copy link
Contributor Author

Hi @manadart. I just added a new test to cover the function. Please take a look 🙂

@skatsaounis skatsaounis requested a review from manadart May 28, 2024 07:13
manadart
manadart reviewed May 31, 2024
View reviewed changes
Copy link
Member

@manadart manadart left a comment

Choose a reason for hiding this comment

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

I think we should avoid modify the existing client method instead of adding a new one.

client.go Outdated Show resolved Hide resolved
@skatsaounis
Copy link
Contributor Author

skatsaounis commented Jun 3, 2024 •
edited
Loading

@manadart Updated as per review. Please re-review 🙂

@skatsaounis skatsaounis requested a review from manadart June 3, 2024 07:09
manadart
manadart approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@manadart manadart 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 seeing this through.

@manadart manadart merged commit 0fb6099 into juju:v2 Jun 7, 2024
@skatsaounis skatsaounis deleted the feat-put-files branch June 10, 2024 06:06
@skatsaounis
Copy link
Contributor Author

skatsaounis commented Jun 10, 2024 •
edited
Loading

Hello @manadart . It's nice that this PR got merged. But in order to consume it properly it needs to be added to a new release. e.g., v2.3.0. Could you please create it in order to consume it from the https://github.com/maas/gomaasclient/? Thanks 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manadart manadart manadart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skatsaounis @jujubot @manadart