Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • Nov 08 22:05

    Fryguy on master

    Optionally symlink spec/managei… Merge pull request #84 from agr… (compare)

  • Nov 08 22:05
    Fryguy closed #84
  • Nov 08 22:05
    Fryguy labeled #84
  • Nov 08 22:05
    Fryguy milestoned #84
  • Nov 08 22:05
    Fryguy assigned #84
  • Nov 08 13:51
    miq-bot commented #84
  • Nov 08 13:50
    agrare synchronize #84
  • Nov 08 13:39
    agrare edited #84
  • Nov 08 13:37
  • Nov 08 13:28
    agrare opened #84
  • Nov 06 17:47
    simaishi assigned #83
  • Nov 06 17:47
    simaishi milestoned #83
  • Nov 06 17:47
    simaishi labeled #83
  • Nov 06 17:47

    simaishi on ivanchuk

    Test ruby 2.5.5, see: https://g… Merge pull request #83 from d-m… (compare)

  • Nov 06 17:47
    simaishi closed #83
  • Nov 06 16:54
    miq-bot commented #83
  • Nov 06 16:43
    miq-bot edited #83
  • Nov 06 16:42
    d-m-u opened #83
  • Oct 30 15:46
  • Oct 25 17:33
    lpichler milestoned #81
Alberto Bellotti
@abellotti
lemme see ....
maybe add a spec that exercises the case where that failing order reason "Service ordering via API is not allowed", other than that LGTM!!
Fabien Dupont
@fdupont-redhat
@abellotti I updated an existing test that was a bit weird: https://github.com/ManageIQ/manageiq-api/blob/707c884ed35865cfc4143826fbc13bb7d363bf0f/spec/requests/service_templates_spec.rb#L638-L649. It tested ordering an unorderable service template, so it didn't even check Settings.product.allow_api_service_ordering. Tell me if that's a good move.
Alberto Bellotti
@abellotti
sorry Fabien, I've been tied up, let me take a look now.
Fabien Dupont
@fdupont-redhat
No problem. Thanks.
Adam Grare
@agrare
Hey @thearifismail @djberg96 looks like the -api specs are failing due to the lan transformation validation PR
can one of you take a look?
thearifismail
@thearifismail
Yes, that's what I am seeing now
in my appliance. let me take a look at it.
Adam Grare
@agrare
:+1: thanks!
thearifismail
@thearifismail
@agrare I can't reproduce the problem locally after syncing my repo with upstream master. That said, there IS something wrong because I was encountering similar problem when using the WebUI. I need to step away but Dan is looking at it
Daniel Berger
@djberg96
i was able to reproduce it locally
Adam Grare
@agrare
On an appliance? I thought it was just a spec failure
Daniel Berger
@djberg96
i think he means a local dev instance
just looks like the lan factories need to actually be attached to a switch/host/ems
Daniel Berger
@djberg96
i think these factories are messed up in general
Daniel Berger
@djberg96
looks like the host types are flip-flopped
Adam Grare
@agrare
oh yeah, source_ems is vmware, source_host is redhat
thearifismail
@thearifismail
I am getting at it
Fabien Dupont
@fdupont-redhat
@abellotti I've been distracted too. PR updated.
Fabien Dupont
@fdupont-redhat
@abellotti thanks :+1:
Martin Hradil
@himdel
Would anyone object to changing API logging so that the requests ends up in rails s output?
(right now, if a GET /dashboard/show fails with 500, rails s output contains the full exception.
if the same happens for an API request, nothing in rails output, you have to look at logs/api.log)
Martin Hradil
@himdel
(correction: you do see the api request, just not the exception)
Martin Hradil
@himdel
no objections I guess :) ManageIQ/manageiq-api#686
Daniel Berger
@djberg96
@abellotti I think the current restriction on ID and HREF might only be on subcollections
it's definitely letting me get away with editing the id field on a single instance
Alberto Bellotti
@abellotti
I removed id from your PR and did a POST /api/regions/3 { "action" : "edit", "id" : "5" }, request goes through, but it did ignore it and stayed 3. Are you seeing this on a different collection ?
Daniel Berger
@djberg96
ah, ok, it just ignores it then, i didn't see if it actually made the change, was only checking for an explicit error
Alberto Bellotti
@abellotti
yeah, not sure if it's the right thing long term. Maybe for API's that rely on the generic edit_resource implementation, it too should throw the BadRequestError if it sees, id, href, created_at or updated_at. This would also simplify your PR (and future controllers).
Daniel Berger
@djberg96
@lpichler @abellotti @martinpovolny looking at zones, it looks like edit/create was explicitly disabled in config/api.yml
Alberto Bellotti
@abellotti
well, just never enabled :) the disabled: true was added so we don't lose info about the identifiers. You can remove the disabled flag once you add those actions.
Daniel Berger
@djberg96
I'm wondering if it would be worth adding an :allow option in the config so you have fine control over what fields can be edited
so disabled: true is mostly true, but we could make an exception for description, for example
without fiddling with the controller
i'm also not sure which fields beyond description we would want to allow users to edit
seems like that could lead to issues, wouldn't it?
Alberto Bellotti
@abellotti
not sure I follow, disabled pertains to the action being available or not. not related to editable attributes. Also, I wouldn't expand too much on that api.yml for editable attributes, moving toward having an OpenAPI spec for our API would be my preference and would open the door to additional integrations.
Daniel Berger
@djberg96
Jason Frey
@Fryguy
Just as a heads up, please be sure that any change is possible in OpenAPI...I'd like to move over to OpenAPI in a soonish release. It will obviously be a major version of the API, but the more special features we add the harder it will be for users to upgrade.
Daniel Berger
@djberg96
ok
djberg96 @djberg96 realizes that openapi is swagger
Jason Frey
@Fryguy
if it's not possible, that's not saying we can't allow the change in, but we should be aware of it
Martin Povolny
@martinpovolny

As @abellotti wrote, disabled means action is disabled. We could probably just remove the disabled actions from the file (while making sure that they are really not implemented)
I am missing the relation to editable attributes.

I think that moving forward we should unify and dry how we handle what is editable and what is not.

Daniel Berger
@djberg96
interesting, i'm getting a 200 response on a failed DELETE in a spec
Martin Povolny
@martinpovolny
@djberg96 : did you resolve that?
(200 response)
Daniel Berger
@djberg96
@martinpovolny not yet