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

Fix issues in Azure SQL branch #170

Merged
merged 30 commits into from
Aug 30, 2019
Merged

Conversation

jananivMS
Copy link
Contributor

What this PR does / why we need it:

  • Fixes issue in Will's code to take location from the spec and not from the config
  • Add nil check to AzureError function to prevent error when printing

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains tests

alexeldeib and others added 30 commits August 6, 2019 22:56
* improve tests with parallel execution and rm sleep

* fix the tests to run on kindcluster
* feat: implement keyvault controller

* Ace's KV changes with updates

* Added an event for the final successful provisioning

* Updated changes based on the PR comments

* removing unwanted file

* making resource group name the one in the keyvault yaml

Co-authored-by: Ace Eldeib <alexeldeib@gmail.com>
* this needs to exist in the reconciler in order to use controllerutil createorupdate
* add consumer group kind

* update tests for consumer group

* fix isbeingdeleted
* cluster additions

* updated docs
* Update azure-pipelines.yaml
* revert back // +kubebuilder:subresource:status changes - fix broken tests
* add dev conatiner - wip

* DevContainer up and running.

* Removed `sleep 80` and replaced with `kubectl wait`.

* Run `make set-kindcluster` from docker-compose.

* Set timeout on wait.

* Added `install-test-dependency` to makefile and dockerfile.

* Update README - Create SP with contribution rights.

* Updated README with details on using devcontainer.

* Stuff that wanted me to commit.

* Reverted changes made to `docker-build` in Makefile.
* first commit on Amanda's branch

* first

* before properties

* test not tested

* test works

* unit tests work, needs firewall rules

* addresses feedback

* erin's feedback

* janani's change, pass future

* async works much better now

* janani feedback

* screwed up interface prototype
* Updated eventhub_types - Added `secretName`.

* Added `secretName` to sample manifest.

* Set secret name to `secretName` if set, otherwise use eventhub name.

* Updated Makefile to update Azure Operator.

Also added the ability to rebuild image without cache.

* Updated README on how to update the Azure Operator.

* Updated CRD with SecretName description.

* Added tests to ensure `SecretName` was being used if present.

* Fix test.
 Please enter the commit message for your changes. Lines starting
…d of config and the error library.

 Please enter the commit message for your changes. Lines starting
Copy link
Contributor

@frodopwns frodopwns left a comment

Choose a reason for hiding this comment

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

lgtm

@jananivMS jananivMS merged commit 0a26b4d into Azure:azure-sql Aug 30, 2019
Porges pushed a commit that referenced this pull request May 11, 2021
* Add StripUnusedDefinitions pass to CodeGenerator.Generate

Update expected output to remove unreferenced types in the expected
output for AllOf_generates_wrapper_type

* Add Referees method to Type and impls

The name is awkward - it might be better to replace References with
this.

* Implement collecting referrers (I think)

* Implement the connectedness check

If a definition isn't connected by a series of references to any root
definition (ie, a resource) then it should be filtered out.

Memoised to save checking a type name twice.

* Correct implementation of TypeName.Referees

Originally implementation was based on a misunderstanding of how the
different astmodel types worked.

* Strip unused definitions in jsonast golden tests

This required moving StripUnusedDefinitions out of pkg/codegen - I put
it into pkg/astmodel because that's where all of the types it
interacts with are. Also enabled passing in a different set of roots,
since the golden tests use the Test type (which isn't a resource) as
the root.

* Update Type.References to return a TypeNameSet, remove Referees

Returning a set makes the old behaviour simple to implement in the few
places that used it.

Adds a SetUnion to combine two TypeNameSets. I thought about making
this a method to add all of the elements of another set to the
receiver - it would be more efficient but I think it's less clean.

* Don't memoise negative checkWithPath results

These can come from a search that is unwinding because it detected a
cycle, rather than one that didn't find a root. In that case we can't
know whether a search from this node with a different path would
succeed.

Added a unit test to explore this problem.

Made the TypeNameSet methods take TypeNames rather than *TypeNames -
this simplified them a bit (at the cost of having to do some extra nil
checks at the edges).

* Corrected grammar on doc comments

* Simplify connectedness-check thanks to a suggestion from Matt

Rather than walking up the references until we find a root, walk down
the references from the roots collecting up all of the types we
reach. This is much simpler and doesn't need any memoisation (since
the accumulated set replaces it).

* Update Function.References to match Type.References

* Remove unnecessary nil checks

These *TypeNames can't be nil - there are a lot of earlier points
in the code that do no-look dereferences.
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

Successfully merging this pull request may close these issues.

7 participants