#808 Ignore mirror_on_sync

[lhellebr]

Parameter mirror_on_sync has been added to API
in Katello PR 9834.
This is not returned as part of JSON on entity creation.
Consequently, read() fails.
It could be workarounded by ignoring errors when reading
non-required parameters. Also, this would be fixed if
Satellite started returning that value.
This solves the issue without unnecessary changes
in Nailgun's guts that might break something.

[codecov]

Codecov Report

Merging #809 (84f8e23) into master (54331ca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #809   +/-   ##
=======================================
  Coverage   95.22%   95.22%           
=======================================
  Files           6        6           
  Lines        2826     2827    +1     
=======================================
+ Hits         2691     2692    +1     
  Misses        135      135           

Impacted Files	Coverage Δ
nailgun/entities.py	94.59% <100.00%> (+<0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54331ca...84f8e23. Read the comment docs.

[jyejare]

ACK

This is exactly where I wanted to introduce 'if issue_is_open' similar to 'if bz_bug_is_open' so that we can skip/run steps based on issue is open or not and we need not to worry about temporary fixing these kinds of issues and rolling it back once its fixed.

[lhellebr]

@jyejare , do you thinks this is a bug in Satellite? I'm not sure myself. Is it reported already? I'm thinking about reporting it.

[Griffin-Sullivan]

I asked about this last week. They replaced mirror_on_sync with mirroring_policy. Here's the PR: Katello/katello#9834

My suggestion would be to remove mirror_on_sync and then add the parameter for mirroring_policy with these options additive, mirror_complete, mirror_content_only.

Like so:

            'mirroring_policy': entity_fields.StringField(
                choices=('additive', 'mirror_complete', 'mirror_content_only'),
            ),

[jyejare]

@Griffin-Sullivan Yeah I remember we talk. So does the mirror_on_sync completely removed (from POST request as well), if it is, we can simply replace mirror_on_sync here with mirroring_policy!

[Griffin-Sullivan]

@jyejare I see both in POST request, but mirror_on_sync says (deprecated). Last week when I was debugging I originally ran into this problem because mirror_on_sync wasn't being returned by mirroring_policy was. So far mirroring_policy in place of mirror_on_sync has worked for me locally.

[jyejare]

@Griffin-Sullivan So does even mirror_on_sync gets executed with POST request successfully and is there any use of it since we have new option for same now?

If there is no use, I will simply replace in this PR !

[Griffin-Sullivan]

@jyejare From my understanding mirroring_policy is replacing mirror_on_sync as it provides more options to the user. In the UI, you will find it fully replaced, but through the API both are still available.... for now. You should be able to use either one right now, but it seems like they are fully replacing mirror_on_sync. I'm going to link the PR to provide more answers: Katello/katello#9834

[lhellebr]

I'm still not sure how to best approach this. But I would merge this PR as is because:

1. mirror_on_sync is still present somewhere, just not in all calls
2. ignoring it makes sense, even more since it's present but deprecated
3. this PR fixes the issue
   We can add mirrorring_policy in a separate PR once we know more.

[Griffin-Sullivan]

@lhellebr I think that's fine, because I'm still working to get mirroring_policy functioning properly. I'm probably doing something wrong, but @jyejare is helping.

[lhellebr]

Let's ACK and merge than?

[jyejare]

@lhellebr Agree with you to ACK and Merge this to avoid related issues in test execution and @Griffin-Sullivan already has PR opened(I think) for mirroring_policy so lets get it in.

[omkarkhatavkar]

ACK