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

Reconciliation loops should be non-blocking #47

Closed
alexeldeib opened this issue Aug 7, 2019 · 4 comments
Closed

Reconciliation loops should be non-blocking #47

alexeldeib opened this issue Aug 7, 2019 · 4 comments

Comments

@alexeldeib
Copy link
Contributor

Currently several reconciliation loops return futures from Azure API calls and wait on their completion. This means deploying many object will cause blocking calls and effectively serialize their creation. This is undesirable.

We should instead make reconciliation not care about the result of the call, allowing it to proceed and pick up any information it would need during the next requeue.

@frodopwns
Copy link
Contributor

Was thinking we send the request without waiting. Then either store the polling url in the entity status and re-queue, or just re-queue.

@alexeldeib
Copy link
Contributor Author

(discussed offline, but for posterity)

easiest pattern is to fire off the request, handle any errors, and then requeue immediately. let the next Get in the reconcile loop update the status to wait for provisioning/deletion.

@jananivMS
Copy link
Contributor

Keep this for now until all of them are fixed

@frodopwns
Copy link
Contributor

We are well on our way to 100% on this if we aren't already

Porges added a commit that referenced this issue May 11, 2021
* wip: walk the deployment schema to harvest types and version

* wip generator to create crds

* Change references to ToPackages() ToNodes()

Fixing a few broken references.

* Ast improvements (#47)

* Fix comment handling to avoid panic

* Define interface AstGenerator

* Create FieldDefinition struct and test

* Create StructDefinition and test

* Factor out loadSchema()

* Introduce SchemaScanner

* Change field factory methods to return FieldDefinition

* Improve logging in root.go

* Make description optional for FieldDefinition

* Modify getFields to return FieldDefinitions

* Relocate TypeHandlers to SchemaScanner

* Add fileDefinition and use to export structs

* Add tests for FileDefinition

* Move filters onto SchemaScanner

* Introduce ScannerTopic and plumb through

* Makes structs versioned

* fixup! Introduce ScannerTopic and plumb through

* Generate structs into different namespaces by version

* Use new interface "Definition" when walking schema tree

* Split method into two better focused methods

* Suppress field comments if empty

* Enforce field names and types being present

Also includes passing ScannerTopic when creating simple fields

* Fix failing tests

* Remove duplicate declaration

* include number of structs written in output message

* Fix detection of version numbers in schema URLs

* Add gitignore

* Switch to log package

* Fix doc-comments (somewhat)

* Fix println with format specifier

* Remove AstGenerator interface

Modify tests to check for the Definition interface instead

* Remove use of panic()

* Remove redundant initialization

* Fix test compilation error

* Return errors from `objectTypeOf()` and `versionOf()`

Co-authored-by: George Pollard <gpollard@microsoft.com>

* Further improvements to AST generation (#52)

* Start code gen from root rather than root/resources

* Remove possibility of failure from ToAst()

There's no reason for this to fail, and it complicates the code.

* Add Type interface to go with Definition

* Flesh out types and update JSONAST to use them

* Alter JSONAST so handlers return Types instead of Definitions

* Add description to root type

* Always use ref URL to generate new topic names

* Document nil return for TypeHandler

This is valid (see, e.g. RefHandler).

* Reorder code to simplify

* Make TypeHandler take scanner explicitly

* Remove use of ScannerTopic which isn't needed any more

* Hide fields in StructType

* Simplify NewSchemaScanner

* Factor out fixedTypeHandler

* Fix nomenclature to match Go

* Reduce nesting in allOfHandler

* Create an IdentifierFactory to create valid Go identifiers (#53)

* Create IdentifierFactory and test

* Hook up use of IdentifierFactory

* Code gardening

* Use gomega instead of testify

* Use idiomatic Go approach for test cases

* Tidy go.mod

* Rename local to idFactory

* Enforce interface implementation w/compiler

* Remove scannerTopic.go

Test coverage increases by 1%!

* Add copyright headers (#59)

* Code Gardening (#58)

* Add missing comments on public declarations

* Whitespaces changes by go fmt

* Preserve immutability by returning a copy of the slice

* Remove else after return

* Add missing header copyright comments

* Fix compilation issues in branch

* Make test pass

* Add support for additionalProperties schema (#55)

* Add support for additionalProperties schema

This lets us generate types like this for tags members:

```go
    	Tags       map[string]string
```

* Add golden-files test infrastructure and tests

This will add coverage for 'additionalProperties'.

* Simplify runGoldenTest file reading

* Run

* Merge 'schema'

Co-authored-by: Dave Fellows <dave.fellows@microsoft.com>
Co-authored-by: Bevan Arps <bevan.arps@outlook.com>
Co-authored-by: George Pollard <gpollard@microsoft.com>
Co-authored-by: Bevan Arps <bevan.arps@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants