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.

Sign up for GitHub

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

Jump to bottom

GET method for ApolloSchemaDownloader #2010

Merged
merged 8 commits into from
Nov 8, 2021
Merged

GET method for ApolloSchemaDownloader #2010

merged 8 commits into from
Nov 8, 2021

Conversation

mikepitre
Copy link
Contributor

@mikepitre mikepitre commented Oct 28, 2021 •
edited
Loading

@apollo-cla
Copy link

@mikepitre: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@mikepitre mikepitre mentioned this pull request Oct 28, 2021
Closed
@calvincestari
Copy link
Member

Thanks for this @mikepitre. I'm going to convert this PR to a draft so we can't merge it until it's ready.

@calvincestari calvincestari marked this pull request as draft October 28, 2021 17:26
@calvincestari calvincestari linked an issue Nov 2, 2021 that may be closed by this pull request
GET option for ApolloSchemaDownloader #1981
Closed
@calvincestari calvincestari changed the base branch from release/1.0-alpha-incubating to main November 2, 2021 21:29
@calvincestari
Minor improvements to HTTP method enum
a0a7e06 
* Remove ApolloSchemaDownload scope from name
* Add documentation
* Add HTTP method string constants as output
@calvincestari
Add error for unsupported HTTP method when using Apollo Registry
32dd79c
@calvincestari
Copy link
Member

I've made some minor changes to the enum and added an error case for trying to use GET with the Apollo Registry. Tomorrow I'll add tests.

@designatednerd
Copy link
Contributor

Worth making this an option on the downloadMethod instead of on the initializer so you can't try to pass this in if you're using the registry?

@calvincestari
Copy link
Member

Worth making this an option on the downloadMethod instead of on the initializer so you can't try to pass this in if you're using the registry?

Yes I'll take a pass at how that would look. There is an error for specifically this, but being able to completely rule out that possibility would be better.

@designatednerd
Copy link
Contributor

Yeah error good, making it actually impossible better 😇

@calvincestari calvincestari marked this pull request as ready for review November 6, 2021 00:05
@calvincestari
Copy link
Member

@designatednerd I think we're good to go with this PR now. There is more that I'd like to refactor with ApolloSchemaDownloadConfiguration but I'll do that in the 1.0 branch (#2012) because it will be breaking changes and right now supporting GET in this PR is all additive.

designatednerd
designatednerd approved these changes Nov 6, 2021
View reviewed changes
Copy link
Contributor

@designatednerd designatednerd left a comment

Choose a reason for hiding this comment

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

Couple suggestions, otherwise good to go!

Sources/ApolloCodegenLib/ApolloSchemaDownloadConfiguration.swift Outdated Show resolved Hide resolved
Sources/ApolloCodegenLib/ApolloSchemaDownloadConfiguration.swift Outdated Show resolved Hide resolved
Sources/ApolloCodegenLib/ApolloSchemaDownloader.swift Outdated Show resolved Hide resolved
@calvincestari calvincestari changed the title Draft: GET method for ApolloSchemaDownloader GET method for ApolloSchemaDownloader Nov 8, 2021
@calvincestari calvincestari changed the title GET method for ApolloSchemaDownloader GET method for ApolloSchemaDownloader Nov 8, 2021
@calvincestari calvincestari merged commit 3bfde02 into apollographql:main Nov 8, 2021
calvincestari added a commit that referenced this pull request Nov 19, 2021
@calvincestari
Squashed merge commit up to 3262cd0
675af6e 
Squashed commit of the following:

commit 3262cd0
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Thu Nov 18 03:49:51 2021 +0000

    fix(deps): update dependency gatsby-theme-apollo-docs to v5.3.8 (#2038)

    Co-authored-by: Renovate Bot <[email protected]>

commit d93568f
Author: Calvin Cestari <[email protected]>
Date:   Wed Nov 17 12:51:58 2021 -0800

    Release `0.50.0` (#2031)

    * Update version number for release
    * Updated changelog for release
    * Update documentation for release changes
    * Update changelog for #2015

commit 46957a3
Author: Hesham Salman <[email protected]>
Date:   Wed Nov 17 15:15:54 2021 -0500

    Update SQLite.swift to version 13.0 (#2015)

    * Update Package.swift and podspec
    * Updated XcodeProj
    * Bump SQLite.swift minimum required version to 0.13.1

    Co-authored-by: Calvin Cestari <[email protected]>

commit 8d48031
Author: hwillson <[email protected]>
Date:   Fri Nov 12 16:24:18 2021 -0500

    Gateway clarification based on license change

commit 55c5db8
Author: Calvin Cestari <[email protected]>
Date:   Fri Nov 12 10:47:33 2021 -0800

    Update apollo-tooling to v2.33.9 (#2028)

    * Update to v2.33.9 of the CLI tooling
    * Update StarWarsAPI output generated by updated tooling
    * Update test with expected output
    * Keep the parameter spacing

commit ece5b5b
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Wed Nov 10 22:13:58 2021 +0000

    fix(deps): update dependency gatsby-theme-apollo-docs to v5.3.6 (#2026)

    Co-authored-by: Renovate Bot <[email protected]>

commit a8a35ae
Author: Anthony Miller <[email protected]>
Date:   Tue Nov 9 11:50:36 2021 -0800

    Attempted fix for integration test failing on CI (#2024)

commit a1e857c
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Nov 8 16:24:21 2021 -0800

    Bump apollo-server from 2.16.1 to 2.25.3 in /SimpleUploadServer (#2020)

    Bumps [apollo-server](https://github.com/apollographql/apollo-server/tree/HEAD/packages/apollo-server) from 2.16.1 to 2.25.3.
    - [Release notes](https://github.com/apollographql/apollo-server/releases)
    - [Changelog](https://github.com/apollographql/apollo-server/blob/main/CHANGELOG.md)
    - [Commits](https://github.com/apollographql/apollo-server/commits/[email protected]/packages/apollo-server)

    ---
    updated-dependencies:
    - dependency-name: apollo-server
      dependency-type: direct:production
    ...

    Signed-off-by: dependabot[bot] <[email protected]>

    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 6bf4362
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Tue Nov 9 00:03:55 2021 +0000

    fix(deps): update dependency gatsby-theme-apollo-docs to v5.3.2 (#2021)

    Co-authored-by: Renovate Bot <[email protected]>

commit 3bfde02
Author: Mike Pitre <[email protected]>
Date:   Mon Nov 8 13:58:36 2021 -0500

    GET method for `ApolloSchemaDownloader` (#2010)

    * GET method for ApolloSchemaDownloader
    * Minor improvements to HTTP method enum
    * Remove ApolloSchemaDownload scope from name
    * Add documentation
    * Add HTTP method string constants as output
    * Add error for unsupported HTTP method when using Apollo Registry
    * Move HTTP method support into DownloadMethod
    * Build requests based on DownloadMethod
    * Add tests for DownloadMethod HTTP method configurations
    * Clean up and clarify documentation
    * Add associated values to URL-related errors

    Co-authored-by: Calvin Cestari <[email protected]>

commit 9cab672
Author: Calvin Cestari <[email protected]>
Date:   Wed Nov 3 12:48:57 2021 -0700

    Expose `cacheKey` function as `public` (#2014)

    * Expose cacheKey function as public
    * Remove @testable attribute to require public access to cacheKey function

commit f2a4983
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Thu Oct 28 01:40:10 2021 +0000

    fix(deps): update dependency gatsby-theme-apollo-docs to v5.3.1 (#2006)

    Co-authored-by: Renovate Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@designatednerd designatednerd designatednerd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GET option for ApolloSchemaDownloader
4 participants
@mikepitre @apollo-cla @calvincestari @designatednerd