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

Bump fog-core version to ~> 2.1 #422

Closed
wants to merge 61 commits into from
Closed

Bump fog-core version to ~> 2.1 #422

wants to merge 61 commits into from

Conversation

gildub
Copy link

@gildub gildub commented Sep 19, 2018

#421

Namespace change to prefix all services/modules with Fog::Google
Also uses "library" path for autoload.

@icco
Copy link
Member

icco commented Sep 19, 2018

This is because of #419

@Temikus Temikus added this to the 2.0 milestone Sep 20, 2018
@Temikus
Copy link
Member

Temikus commented Sep 20, 2018

I'm not quite sure why upgrade is required so asked some questions on the issue, deferring until then.

lib/fog/google/storage/storage_json.rb Outdated Show resolved Hide resolved
lib/fog/google/storage/storage_json.rb Outdated Show resolved Hide resolved
lib/fog/google/storage/storage_json.rb Outdated Show resolved Hide resolved
lib/fog/google/monitoring.rb Outdated Show resolved Hide resolved
lib/fog/google/monitoring.rb Outdated Show resolved Hide resolved
lib/fog/google/pubsub.rb Outdated Show resolved Hide resolved
lib/fog/google/pubsub.rb Outdated Show resolved Hide resolved
lib/fog/google/parsers/storage/storage.rb Outdated Show resolved Hide resolved
lib/fog/google/parsers/storage/storage.rb Outdated Show resolved Hide resolved
lib/fog/google/parsers/storage/storage.rb Outdated Show resolved Hide resolved
lib/fog/google/compute/compute.rb Outdated Show resolved Hide resolved
lib/fog/google/compute/compute.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
lib/fog/google.rb Outdated Show resolved Hide resolved
@Temikus
Copy link
Member

Temikus commented Sep 21, 2018

@gildub Thank you for rewriting the references, I really appreciate it.

However, I rolled back the tests to see if the interface is compatible and it's not, so this PR introduced a major breaking change. A major breaking change in the interface requires bumping up the major version so I'll defer it until 2.0 which I plan to release in the coming months.

As a workaround you can lock your fog-google dependency to v1.7.1 - it doesn't lock fog-core is quite stable and should provide all the API interfaces you need for now, even though it may produce some deprecation warnings.

I hope you understand where I'm coming from as we have a contract to follow semantic versioning with our userbase.

CC/ icco@ in case I'm missing something as it's good to have another pair of eyes on the problem.

@gildub
Copy link
Author

gildub commented Sep 21, 2018

@Temikus, I absolutely understand the constraints implied by the interface changes.

The reason behind all of this is because fog-openstack needs fog/fog-core@939efc1 which is in the same release as the namespace change.

All fog projects have to face the equivalent API changes due to the renaming. Since both namespaces can't be supported at same time, having 2 versions is likely the best approach. So if consumers don't want any disruption they keep using the older one (fog-google ~> 1.7) while the others can use (~> 2.x).

The later would be ideal, otherwise we'll wait for fog-google release version 2, along with fog-openstack to work around fog-core 2.1.0 using a monkey patch. And hoping this would be sooner than later, as other fog projects are moving forward (partly my fault as I've been pushing to get passed fog-core 2.1.0).

What do you think?

@Temikus
Copy link
Member

Temikus commented Sep 22, 2018

@gildub I'll try to get 2.0 out of the door sooner. Additionally, I'll take a look at whether we can intercept and switch the namespaces and push this out without disrupting users.

Sorry I cannot alleviate your pain at this very moment, but we do need to keep to our versioning.

@gildub
Copy link
Author

gildub commented Sep 24, 2018

@Temikus,

Looking forward to get 2.0. Thanks for following up.

@gildub gildub changed the title Bump fog-core version to 2.1.2 Bump fog-core version to ~> 2.1.2 Oct 2, 2018
@gildub gildub changed the title Bump fog-core version to ~> 2.1.2 Bump fog-core version to ~> 2.1 Oct 2, 2018
@github-actions
Copy link

This pr has been marked inactive and will be closed if no further activity occurs.

@brwainer
Copy link

still needed.

@Temikus
Copy link
Member

Temikus commented Sep 21, 2021

Now that we have CI - @icco do you have some bandwidth to take a look by any chance?

@github-actions
Copy link

This pr has been marked inactive and will be closed if no further activity occurs.

@utkarsh2102
Copy link

This pr has been marked inactive and will be closed if no further activity occurs.

.

@github-actions
Copy link

This pr has been marked inactive and will be closed if no further activity occurs.

@utkarsh2102
Copy link

This pr has been marked inactive and will be closed if no further activity occurs.

.

sethboyles added a commit to cloudfoundry/cloud_controller_ng that referenced this pull request Jan 25, 2022
- Bumps fog-core to 2.1.2 instead of 2.2.4 because the latest version of
fog-google is incompatible with fog-core > 2.1.0
- Pin fog-google to 1.7.1 based [a
recommendation](fog/fog-google#422 (comment)) from one of the
fog-google maintainers.
- Temporarily disables fog deprecation warnings when loading in the
various fog gems because several of them haven't updated all their
modules/classes to use the new Fog::<provider>::<service> structure.

[#180880562]

Co-authored-by: Tom Viehman <tviehman@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
sethboyles added a commit to cloudfoundry/cloud_controller_ng that referenced this pull request Jan 26, 2022
- Bumps fog-core to 2.1.2 instead of 2.2.4 because the latest version of
fog-google is incompatible with fog-core > 2.1.0
- Pin fog-google to 1.7.1 based [a
recommendation](fog/fog-google#422 (comment)) from one of the
fog-google maintainers.
- Temporarily disables fog deprecation warnings when loading in the
various fog gems because several of them haven't updated all their
modules/classes to use the new Fog::<provider>::<service> structure.

[#180880562]

Co-authored-by: Tom Viehman <tviehman@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
sethboyles added a commit to cloudfoundry/cloud_controller_ng that referenced this pull request Jan 26, 2022
- Bumps fog-core to 2.1.2 instead of 2.2.4 because the latest version of
fog-google is incompatible with fog-core > 2.1.0
- Pin fog-google to 1.7.1 based [a
recommendation](fog/fog-google#422 (comment)) from one of the
fog-google maintainers.
- Temporarily disables fog deprecation warnings when loading in the
various fog gems because several of them haven't updated all their
modules/classes to use the new Fog::<provider>::<service> structure.

[#180880562]

Co-authored-by: Tom Viehman <tviehman@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
@sethboyles
Copy link
Contributor

@Temikus @geemus is there any way we can help move this along? I see tests are passing on the CI--what else needs to be done before this can be merged?

@geemus
Copy link
Member

geemus commented Jan 30, 2022

@sethboyles Hey, will try to do what I can. I'm playing catch up a bit, so apologies if I miss anything. I'll try to share my understanding/thoughts and support.

First off, if I recall properly the update to fog-core 2.1+ should (at least in theory) be doable without it being a breaking change (otherwise it would have been fog-core 3.0 itself). That being said, it doesn't mean it's as easy or painless as we might have hoped. So, first things first. I think it's possible to simply do the update and have most everything still technically work, albeit littered with annoying deprecation messages.

Still, that might be a good first step, and I believe the deprecation messages can be disabled until we could play catch-up on renaming/moving things (which shouldn't be the hardest thing, but it is pretty tedious and unfortunately a bit error-prone in my experience). Maybe we could talk about doing the upgrade (and temporarily allowing the deprecation messages) to get folks unstuck?

This also means that the deprecation stuff can be fixed bit-by-bit, which is probably much nicer than trying to do it wholesale (again, tedious and error-prone). This could also facilitate doing some divide-and-conquer if we coordinate, so no one person has to do the whole lift.

Finally, I think it's possible, with some minor workarounds, to do the renaming/moving and have it be non-breaking. I'm pretty sure we managed it in some of the other providers at least (if memory serves), but it's not the most obvious. I think one of the key parts is creating aliases to the old style, which have their own deprecation warnings. This basically shifts the interface change from being a library-level issue to a user-level issue (so they can still be warned and update their code).

You can see an example of this technique from fog-aws here:

https://github.com/fog/fog-aws/blob/2a3fe7ae0e4947fdeef25724456b4d0c1ece821a/lib/fog/aws/storage.rb#L822-L834

Does that all make sense? I think we could make that kind of shift/adjustment bit by bit then until the deprecations only remain for those whose implementation lags (instead of being a library-level issue).

My historical desire to move us to consistency was good, but I admit it left some pain and confusion in it's wake.

I haven't closely followed all the fog libraries in some time, and probably won't (there are SO many and I haven't used fog myself in years), but I'm always available to offer advice and support. So don't hesitate to grab me if things like this come up that need some guidance.

@geemus
Copy link
Member

geemus commented Jan 30, 2022

@Temikus Does that help? I saw you were concerned about breaking changes, but I think maybe they can be worked-around with a bit of effort. I provided one example above, but if there are other outliers just let me know and I can see if we have examples (or I can come up with ideas) for how to avoid them so that we can move this forward without a major.

@sethboyles
Copy link
Contributor

@geemus @Temikus I made a PR that just bumps fog-core here: #562. That should be less overwhelming to merge in.

@Temikus
Copy link
Member

Temikus commented Feb 13, 2022

@geemus thanks, that's the bit that I was missing. I'm happy to move forward here with the same method that fog-aws used.

I admit that I have a bit less time to maintain things nowadays as Google decided to discontinue sponsorship of this project (we would have no CI even if it wasn't for a very kind gesture on behalf of MeindMeister ❤️ ), so I have to maintain this project in my free time, which is currently limited as I'm very focused on $DAYJOB.

I think I'll merge the change @sethboyles is proposing, just need to see what's failing in the integration tests.

@geemus
Copy link
Member

geemus commented Feb 13, 2022

I think we should close this now, unfortunately. The bump is now merged via #562, and the other changes here have fallen out of sync and have conflicts. So, given that state of things, I will close and we can revisit things more bit-by-bit hopefully.

@geemus geemus closed this Feb 13, 2022
@geemus
Copy link
Member

geemus commented Feb 13, 2022

@Temikus No worries, I appreciate the help you have given and whatever support you can continue to give, but I definitely understand that life situations change. Whatever help you can still give will be great, but no worries if not. I can certainly try to take up some of the slack as I'm able (also in somewhat limited time that I have).

will-gant pushed a commit to sap-contributions/cloud_controller_ng that referenced this pull request Dec 16, 2022
- Bumps fog-core to 2.1.2 instead of 2.2.4 because the latest version of
fog-google is incompatible with fog-core > 2.1.0
- Pin fog-google to 1.7.1 based [a
recommendation](fog/fog-google#422 (comment)) from one of the
fog-google maintainers.
- Temporarily disables fog deprecation warnings when loading in the
various fog gems because several of them haven't updated all their
modules/classes to use the new Fog::<provider>::<service> structure.

[#180880562]

Co-authored-by: Tom Viehman <tviehman@pivotal.io>
Co-authored-by: Seth Boyles <sboyles@pivotal.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
2.0 Release
  
Backlog
Development

Successfully merging this pull request may close these issues.

None yet