Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Spring Cloud Config and Bus over Pub/Sub sample/docs #1550

Merged
merged 26 commits into from
Apr 1, 2019

Conversation

elefeint
Copy link
Contributor

Because Spring Cloud Bus integrates with Spring Cloud Stream, bus over Pub/Sub is already supported with our current spring-cloud-stream-binder module.

This PR contains a sample demonstrating storing configuration on a local file system, and storing it on GitHub (or any git repo, really).

There will be a follow up with a bit of custom code for supporting configuration change notifications from Cloud Source Repositories -- those would come over Pub/Sub instead of web hook invocation to /monitor.

@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #1550 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1550   +/-   ##
=========================================
  Coverage     68.66%   68.66%           
  Complexity     1683     1683           
=========================================
  Files           244      244           
  Lines          6526     6526           
  Branches        662      662           
=========================================
  Hits           4481     4481           
  Misses         1754     1754           
  Partials        291      291
Flag Coverage Δ Complexity Δ
#unittests 68.66% <ø> (ø) 1683 <ø> (ø) ⬇️

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 9307d51...8b492f5. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 19, 2019

Codecov Report

Merging #1550 into master will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1550      +/-   ##
============================================
+ Coverage     68.58%   68.67%   +0.09%     
- Complexity     1692     1705      +13     
============================================
  Files           249      249              
  Lines          6601     6678      +77     
  Branches        667      676       +9     
============================================
+ Hits           4527     4586      +59     
- Misses         1778     1795      +17     
- Partials        296      297       +1
Flag Coverage Δ Complexity Δ
#integration ? ?
#unittests 68.67% <ø> (+0.09%) 1705 <ø> (+13) ⬆️
Impacted Files Coverage Δ Complexity Δ
.../data/datastore/core/DatastoreResultsIterable.java 90% <0%> (-10%) 4% <0%> (ø)
...store/repository/query/AbstractDatastoreQuery.java 60.86% <0%> (-5.8%) 5% <0%> (ø)
...oud/gcp/data/datastore/core/DatastoreTemplate.java 89.61% <0%> (-1.25%) 112% <0%> (ø)
...ta/datastore/core/convert/TwoStepsConversions.java 88.12% <0%> (-0.08%) 61% <0%> (ø)
...store/repository/query/PartTreeDatastoreQuery.java 81.98% <0%> (-0.07%) 48% <0%> (+1%)
...n/java/com/example/DatastoreRepositoryExample.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
.../datastore/repository/query/GqlDatastoreQuery.java 71.11% <0%> (+12.24%) 20% <0%> (+11%) ⬆️

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 fe1eba4...32bc8e9. Read the comment docs.

@elefeint elefeint requested a review from meltsufin March 19, 2019 18:44
Copy link
Contributor

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Just a couple minor concerns.

Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

I feel like you have such good information in the sample. Maybe we can copy most of it into the refdoc as well?
Also, the naming of this feature is very confusing. Can we just call it "Spring Cloud Bus", or it's too generic to mean configuration management? From everything I've seen about Spring Cloud Bus, no one ever talks about anything but managing configuration with it.

elefeint and others added 14 commits March 20, 2019 11:19
This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener).

# Fixed URLs

## Fixed Success
These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended.

* [ ] http://www.apache.org/licenses/ with 1 occurrences migrated to:
  https://www.apache.org/licenses/ ([https](https://www.apache.org/licenses/) result 200).
* [ ] http://www.apache.org/licenses/LICENSE-2.0 with 504 occurrences migrated to:
  https://www.apache.org/licenses/LICENSE-2.0 ([https](https://www.apache.org/licenses/LICENSE-2.0) result 200).
Co-Authored-By: elefeint <41136058+elefeint@users.noreply.github.com>
This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener).

# HTTP URLs that Could Not Be Fixed
These URLs were unable to be fixed. Please review them to see if they can be manually resolved.

* [ ] http://xslthl.sourceforge.net/ (200) with 1 occurrences could not be migrated:
   ([https](https://xslthl.sourceforge.net/) result AnnotatedConnectException).

# Fixed URLs

## Fixed But Review Recommended
These URLs were fixed, but the https status was not OK. However, the https status was the same as the http request or http redirected to an https URL, so they were migrated. Your review is recommended.

* [ ] http://maven.apache.org/POM/4.0.0 (404) with 114 occurrences migrated to:
  https://maven.apache.org/POM/4.0.0 ([https](https://maven.apache.org/POM/4.0.0) result 404).

## Fixed Success
These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended.

* [ ] http://maven.apache.org/xsd/maven-4.0.0.xsd with 5 occurrences migrated to:
  https://maven.apache.org/xsd/maven-4.0.0.xsd ([https](https://maven.apache.org/xsd/maven-4.0.0.xsd) result 200).
* [ ] http://www.w3.org/2001/XMLSchema-instance with 57 occurrences migrated to:
  https://www.w3.org/2001/XMLSchema-instance ([https](https://www.w3.org/2001/XMLSchema-instance) result 200).
artembilan
artembilan previously approved these changes Mar 28, 2019
Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

This is really nice! Sorry for the bunch of small nits.

Co-Authored-By: elefeint <41136058+elefeint@users.noreply.github.com>
}
----

=== Configuration Management with Spring Cloud Config
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this Spring Cloud Config + Spring Cloud Bus?

Also, you don't really mention the Spring Cloud Stream binder for Pub/Sub.
All of the configuration for the Pub/Sub binder should apply here too, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spring Cloud Bus is the top level header, but I'll add it here too for searchability.

For stream binder -- I mention it in the sample; I'll add here, too.

meltsufin
meltsufin previously approved these changes Apr 1, 2019
meltsufin
meltsufin previously approved these changes Apr 1, 2019
@elefeint elefeint merged commit 694f812 into master Apr 1, 2019
@meltsufin meltsufin deleted the pubsub-cloud-config branch July 25, 2019 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants