pepper icon indicating copy to clipboard operation
pepper copied to clipboard

Use tgt_type instead of expr_form

Open the-glu opened this issue 6 years ago • 13 comments

expr_form was deprecated in 2017.07 and removed in latest salt release (2019.02).

There was a partial fix in #143, but that didn't fix low api calls - right now pepper and this option cannot be used again a 2019.02 server.

This PR fix this, but without any backward compatibility.

the-glu avatar Apr 02 '19 07:04 the-glu

Hi, I got error with salt 2018.3.2 (Oxygen) using client_batch and expr_form.

salt_api.low([{'tgt': '*', 'batch': '1', 'client': 'local_batch', 'arg': 'sleep 5; date', 'fun': 'cmd.run', 'expr_form': 'glob'}]
... )
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/user/.local/lib/python2.7/site-packages/pepper/libpepper.py", line 298, in low
    return self.req(path, lowstate)
  File "/home/user/.local/lib/python2.7/site-packages/pepper/libpepper.py", line 242, in req
    raise PepperException('Server error.')
pepper.exceptions.PepperException: Server error.

With tgt_type it works perfectly.

kovacsbalu avatar Feb 14 '20 12:02 kovacsbalu

Howdy! :wave:

Apologies for the long delay on this integrating this code. We certainly appreciate your effort!

Pepper is getting some renewed focus and love, and we're checking in to see if you'd like to update this against the latest version of the base branch and ensure that all merge conditions are met.

If the items addressed by this contribution have already been fixed/added, are no longer an issue, or you no longer have a desire to work on this... please let us know and/or close out this PR.

If we don't hear from you within 7 days, we'll assume the code has been abandoned and close it out. If you'd like to revisit this in the future, you can always open a new PR.

Thank you, and we hope to see you active in the Salt ecosystem in the future!

nicholasmhughes avatar Apr 20 '23 19:04 nicholasmhughes

I'm not the author of this PR but I can confirm that this fix is required for interacting with recent releases of Salt. Thanks!

enricotagliani avatar Apr 20 '23 20:04 enricotagliani

I seem I missed the comments two years ago, sorry about that.

I will update the PR with comment as soon as possible, probably next week :)

the-glu avatar Apr 20 '23 20:04 the-glu

Hello,

I added backward compatiblity + some tests, however I'm not sure how to tests test - there is no Travis CI there ?

the-glu avatar Apr 24 '23 08:04 the-glu

Hello,

I added backward compatiblity + some tests, however I'm not sure how to tests test - there is no Travis CI there ?

Thanks for the updates and adding tests. We're currently moving the tests to Github Actions (see #226 and #227), but we're waiting on some help from vmware folks to get privs to alter the PR workflow. I'll let you know once that's sorted - thanks for our patience.

barneysowood avatar Apr 25 '23 12:04 barneysowood

Hello, I added backward compatiblity + some tests, however I'm not sure how to tests test - there is no Travis CI there ?

Thanks for the updates and adding tests. We're currently moving the tests to Github Actions (see #226 and #227), but we're waiting on some help from vmware folks to get privs to alter the PR workflow. I'll let you know once that's sorted - thanks for our patience.

@the-glu - just to let you know that I've now merged the updated test suite that will use GH Actions. Could you update your branch?

A brief look at your tests suggests they should still work without any changes, but if you have any issues please let me know - happy to help out.

barneysowood avatar May 02 '23 17:05 barneysowood

@the-glu - just to let you know that I've now merged the updated test suite that will use GH Actions. Could you update your branch?

A brief look at your tests suggests they should still work without any changes, but if you have any issues please let me know - happy to help out.

@barneysowood Done, but it does seem you have to approve the actions :)

the-glu avatar May 02 '23 18:05 the-glu

Ok it does seem to works, I fix tests asap

the-glu avatar May 02 '23 18:05 the-glu

Hi,

I've approved those - I think that's just because you merged in my changes that had GH actions changes...

Thanks!

On Tue, 2 May 2023, at 7:27 PM, Maximilien Cuony wrote:

@the-glu https://github.com/the-glu - just to let you know that I've now merged the updated test suite that will use GH Actions. Could you update your branch?

A brief look at your tests suggests they should still work without any changes, but if you have any issues please let me know - happy to help out.

@barneysowood https://github.com/barneysowood Done, but it does seem you have to approve the actions :)

— Reply to this email directly, view it on GitHub https://github.com/saltstack/pepper/pull/186#issuecomment-1531949512, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG7B5ON4VCQVRI5TW2ZNM3XEFG27ANCNFSM4HC5PE7Q. You are receiving this because you were mentioned.Message ID: @.***>

-- Barney Sowood @.***

barneysowood avatar May 02 '23 19:05 barneysowood

Ok I fixed tests, PR should be ok now :)

the-glu avatar May 03 '23 09:05 the-glu

Hi @barneysowood, is there any chance this will be merged soon? This PR fixes a major incompatibility with the current version of salt

os-sengel avatar Sep 05 '23 07:09 os-sengel

@barneysowood not having this merged is preventing us from using pepper currently

timwsuqld avatar Nov 01 '23 14:11 timwsuqld