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

Batch: Data plane TypeSpec #37319

Merged
merged 98 commits into from
May 14, 2024
Merged

Batch: Data plane TypeSpec #37319

merged 98 commits into from
May 14, 2024

Conversation

skapur12
Copy link
Member

@skapur12 skapur12 commented Oct 23, 2023

Remove ocp date header, add options bag support, add createTasks method, and fix minor typos

Description

Major fixes:

Ocp Date Header

  • Remove ocp date header from SDK and update SDK and unit tests accordingly

Options Bag:

  • Group optional arguments into an 'options bag', add OptionsBag class, and update Batch sync and async clients to use this Options Bag
  • Update TypeSpec with renaming changes from Dave (mostly renaming Options to Parameters and prefixing things with Batch and regenerated SDK
  • Add Internal tag to the end of Java methods and regenerated SDK
  • In sync and async clients: Wrap each generated method with Internal in its name in a handwritten method without the Internal tag (for example, enableJobInternal is wrapped by enableJob). Did this for methods with and without options bags (for example, enableJob(String jobId) and enableJob(String jobId, EnableBatchJobOptions options))
  • In sync and async clients: For every generated method ending in InternalWithResponse (e.g. getApplicationInternalWithResponse), add a handwritten method without the Internal tag that takes in the same parameters and returns that internal method (e.g. getApplicationWithResponse returns this.getApplicationInternalWithResponse)

createTasks Async method:

  • Added createTasks method to the BatchAsyncClient
  • Abstracted away createTasks logic for the sync and async clients into custom classes in task directory

Tests:

  • Update Track 2 tests to reflect the renaming, options bag changes, and added new test for createTasks method
  • Successfully re-ran tests

Minor fixes:

  • Fix timeOut description in TypeSpec and update SDK accordingly
  • Fix typo in extensionName property and update SDK accordingly
  • Address all feedback from latest revision (rev 5)- including renames and other changes:

For more context on the options bag change, see: https://apiview.dev/Assemblies/Conversation/39c0b0984fcc45ebae23d581e3afc9ed


Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@skapur12 skapur12 changed the title Add options bag support and fix minor typos Remove ocp date header, add options bag support, and fix minor typos Oct 23, 2023
Copy link
Member

@dpwatrous dpwatrous left a comment

Choose a reason for hiding this comment

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

(See the other comments)

@dpwatrous dpwatrous self-requested a review November 7, 2023 17:11
Copy link
Member

@dpwatrous dpwatrous left a comment

Choose a reason for hiding this comment

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

Nit: Can we rename BatchServiceClientTestBase to BatchClientTestBase?

@dpwatrous dpwatrous self-requested a review November 14, 2023 16:53
@dpwatrous dpwatrous self-requested a review November 17, 2023 20:52
Copy link
Member

@dpwatrous dpwatrous left a comment

Choose a reason for hiding this comment

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

Looks good - just a couple of very minor comments.

@skapur12 skapur12 force-pushed the skapur/batch-options-bag branch from 1d6f6ff to 596c2ed Compare May 12, 2024 21:44
@dpwatrous dpwatrous self-requested a review May 14, 2024 17:36
@skapur12 skapur12 merged commit 2fcd786 into main May 14, 2024
21 checks passed
@skapur12 skapur12 deleted the skapur/batch-options-bag branch May 14, 2024 19:15
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.

5 participants