Convert most AMQP types to read-only; Significant API surface cleanup. #4640

LarryOsterman · 2023-05-17T00:50:14Z

Significant cleanup of AMQP internals.

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

…ation

…et for message sender and receiver

LarryOsterman · 2023-05-17T19:49:31Z

Ping :).

…for message sender and receiver

… connections; sender and receiver don't use URLs in samples any more; all samples key off environment variables created by new-testresources.ps1

LarryOsterman · 2023-05-22T22:48:25Z

Ping :).

LarryOsterman · 2023-05-23T18:04:52Z

Ping 2 :)

…d the eventhub that was created

antkmsft

Partial review, only 4 files so far.

sdk/core/azure-core-amqp/README.md

sdk/core/azure-core-amqp/inc/azure/core/amqp/connection.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/connection_string_credential.hpp

Co-authored-by: Anton Kolesnyk <[email protected]>

…dded some timeouts to send async test

…com/LarryOsterman/azure-sdk-for-cpp into larryo/readonlyconnectionandsession

antkmsft

10 more files reviewed

sdk/core/azure-core-amqp/inc/azure/core/amqp/endpoint.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/link.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/message_receiver.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/models/message_source.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/models/message_target.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/network/amqp_header_detect_transport.hpp

antkmsft

7 more files, bringing the total to 21/70 files reviewed (30%).

sdk/core/azure-core-amqp/inc/azure/core/amqp/network/sasl_transport.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/network/socket_transport.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/network/tls_transport.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/network/transport.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/session.hpp

sdk/core/azure-core-amqp/samples/CMakeLists.txt

antkmsft

Reviewed 14 more files, bringing the total to 35/70 files reviewed (50%).

sdk/core/azure-core-amqp/samples/eventhub_async_writer_sample/eventhub_async_writer_sample.cpp

sdk/core/azure-core-amqp/samples/eventhub_get_eventhub_properties_sample/CMakeLists.txt

sdk/core/azure-core-amqp/samples/eventhub_token_reader_sample/eventhub_token_reader_sample.cpp

sdk/core/azure-core-amqp/samples/eventhub_token_writer_sample/eventhub_token_writer_sample.cpp

sdk/core/azure-core-amqp/samples/local_server_sample/local_server_sample.cpp

antkmsft

Reviewed 14 more files, bringing the total to 35/70 files reviewed (50%).

sdk/core/azure-core-amqp/inc/azure/core/amqp/endpoint.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/management.hpp

sdk/core/azure-core-amqp/samples/local_server_sample/local_server_sample.cpp

antkmsft · 2023-05-24T20:43:01Z

@LarryOsterman, BTW, if samples do show the usage for the Azure SDK developers, then they can use Core::_internal::Environment, the same way they also do use Amqp::_internal, i.e. once you are user of _internal, the entire _internal surface is available to you. Other thing is if getenv() in samples plays purely a helper role and not the intended use case that we would like to emphasize. Then both are fine.

sdk/core/azure-core-amqp/inc/azure/core/amqp/connection.hpp

LarryOsterman · 2023-05-24T21:35:00Z

@LarryOsterman, BTW, if samples do show the usage for the Azure SDK developers, then they can use Core::_internal::Environment, the same way they also do use Amqp::_internal, i.e. once you are user of _internal, the entire _internal surface is available to you. Other thing is if getenv() in samples plays purely a helper role and not the intended use case that we would like to emphasize. Then both are fine.

getenv() in these samples plays the exact same role as it does in all the other samples: It allows external configuration of the samples to work with the service.

antkmsft

44/88 files reviewed (new 50%)

sdk/core/azure-core-amqp/inc/azure/core/amqp/management.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/message_receiver.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/message_sender.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/message_receiver.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/models/message_target.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/models/messaging_values.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/session.hpp

antkmsft

Thank you! - And sorry it took so long.

sdk/core/azure-core-amqp/inc/azure/core/amqp/connection.hpp

sdk/core/azure-core-amqp/inc/azure/core/amqp/models/messaging_values.hpp

...ure-core-amqp/samples/internal/eventhub_async_writer_sample/eventhub_async_writer_sample.cpp

...ure-core-amqp/samples/internal/eventhub_token_reader_sample/eventhub_token_reader_sample.cpp

sdk/core/azure-core-amqp/src/amqp/message_receiver.cpp

sdk/core/azure-core-amqp/src/amqp/private/message_sender_impl.hpp

sdk/core/azure-core-amqp/test/ut/connection_tests.cpp

* Remove -Login and -PushImages parameters from stress helper (#4615) * Add Azure OpenAI (#4644) Co-authored-by: Shawn Fang <[email protected]> * move the cleanup config to -SaveInputs parameter on the cli (#4646) Co-authored-by: m-nash <[email protected]> * Fixes for uploading proxy logs (#4652) Co-authored-by: Bill Wert <[email protected]> * Handle docker hangs and subscription mismatch on acr login (#4653) Co-authored-by: Ben Broderick Phillips <[email protected]> * Sync eng/common directory with azure-sdk-tools for PR 6202 (#4651) * Adding initial commit of TypeSpecE2E common script * add TODO comment * Updated per feedback --------- Co-authored-by: raychen <[email protected]> * added a test helper to test each type for having the expected default operations. (#4627) * add test helper * constructible from Args * restucture ClassTraits to ensure portability of static constexpr * fix Etags tests and format * clang format * Add DefaultAzureCredential auth flow diagram (#4654) * Add DefaultAzureCredential auth flow diagram * Update cspell.json --------- Co-authored-by: Anton Kolesnyk <[email protected]> * Remove defunct template (#4659) Co-authored-by: James Suplizio <[email protected]> * Convert most AMQP types to read-only; Significant API surface cleanup. (#4640) * Prep for readonly connection * Added options for Connection and Session to enable read-only configuration * Read-only connection and session objects; use message source and target for message sender and receiver * Enabled read-only session; added tests for incorrect AMQPS port * Added token reader sample; flushed out eventhubs properties sample, added some timeouts to send async test * Moved AMQP samples to internal * Moved samples to use GetEnvHelper --------- Co-authored-by: Anton Kolesnyk <[email protected]> * Define a standardized header ordering for Azure SDK for C++ (#4632) * Standardized header ordering for C++ repo * Reordered categories to move private headers above public headers; added space between categories to enable future flexibility * fix flaky storage testcase (#4660) --------- Co-authored-by: Ben Broderick Phillips <[email protected]> Co-authored-by: Azure SDK Bot <[email protected]> Co-authored-by: Shawn Fang <[email protected]> Co-authored-by: m-nash <[email protected]> Co-authored-by: Bill Wert <[email protected]> Co-authored-by: raychen <[email protected]> Co-authored-by: Jennifer Chukwu <[email protected]> Co-authored-by: Scott Addie <[email protected]> Co-authored-by: Anton Kolesnyk <[email protected]> Co-authored-by: James Suplizio <[email protected]> Co-authored-by: Larry Osterman <[email protected]> Co-authored-by: JinmingHu <[email protected]>

LarryOsterman added 11 commits May 16, 2023 17:12

Prep for readonly connection

b749504

Added options for Connection and Session to enable read-only configur…

70ecf68

…ation

Read-only connection and session objects; use message source and targ…

9d51ba5

…et for message sender and receiver

Enabled read-only session; added tests for incorrect AMQPS port

2d2d7af

Cleanup of objects

15cd3ab

gcc9 fixes

d9fe28e

gcc9 fixes 2

4724a20

gcc9 fixes 3

c405933

gcc9 fixes 3

f434991

gcc9 fixes 4

3661f19

gcc9 fixes 5

a972875

LarryOsterman requested review from RickWinter, ahsonkhan, antkmsft, vhvb1989 and gearama as code owners May 17, 2023 00:50

github-actions bot added the Azure.Core label May 17, 2023

LarryOsterman added 3 commits May 16, 2023 18:07

clang-format; cspell

ec67c2a

doxygen

08b51d7

doxygen 2

11b8f64

LarryOsterman added 7 commits May 17, 2023 15:28

Fixed line accidentally removed

3a6384b

Sas credentials are azure credentials; removed sas credential option …

23b81e8

…for message sender and receiver

Moved credentials to session; reworked how audience is calculated for…

68d2bbb

… connections; sender and receiver don't use URLs in samples any more; all samples key off environment variables created by new-testresources.ps1

clang-format

fe0e7dd

clang fix

da97d9c

CI fixes

ee17b57

Mac doesn't talk sockets

b72743b

LarryOsterman added 2 commits May 23, 2023 13:26

Added test-resources.json for samples to ease deployment

2f9ef28

Updated test-resources.json to grant the test user the ability to rea…

28028c5

…d the eventhub that was created

antkmsft reviewed May 24, 2023

View reviewed changes

LarryOsterman and others added 4 commits May 23, 2023 17:31

Update sdk/core/azure-core-amqp/README.md

e723b99

Co-authored-by: Anton Kolesnyk <[email protected]>

Added token reader sample; flushed out eventhubs properties sample, a…

8e1bf8b

…dded some timeouts to send async test

Merge branch 'larryo/readonlyconnectionandsession' of https://github.…

72ef244

…com/LarryOsterman/azure-sdk-for-cpp into larryo/readonlyconnectionandsession

PUll request feedback

875c890

antkmsft reviewed May 24, 2023

View reviewed changes

LarryOsterman added 2 commits May 24, 2023 12:30

More PR feedback

604d4ce

PR feedback

e956dc6

antkmsft reviewed May 24, 2023

View reviewed changes

LarryOsterman added 2 commits May 24, 2023 13:14

Moved AMQP samples to internal

6cd6401

Moved samples to use GetEnvHelper

c2ced30

antkmsft reviewed May 24, 2023

View reviewed changes

sdk/core/azure-core-amqp/inc/azure/core/amqp/connection.hpp Outdated Show resolved Hide resolved

LarryOsterman added 3 commits May 24, 2023 15:44

PR feedback; Don't poll when connection is in the 'End' state

e535e60

clang-format

850d0e6

Improved diagnostics for cbs open to help detect CBS errors in tests

35c1706

antkmsft reviewed May 25, 2023

View reviewed changes

More pull request feedback - removed lots of fully qualified namespaces

1110c0c

antkmsft approved these changes May 26, 2023

View reviewed changes

LarryOsterman added 3 commits May 26, 2023 11:08

more pull request feedback

0c73559

clang-format

8c8277e

Prevent creation of Messaging static-only class

0759b79

LarryOsterman merged commit 99b1ef5 into Azure:main May 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert most AMQP types to read-only; Significant API surface cleanup. #4640

Convert most AMQP types to read-only; Significant API surface cleanup. #4640

LarryOsterman commented May 17, 2023

LarryOsterman commented May 17, 2023

LarryOsterman commented May 22, 2023

LarryOsterman commented May 23, 2023

antkmsft left a comment

antkmsft left a comment

antkmsft left a comment

antkmsft left a comment

antkmsft left a comment

antkmsft commented May 24, 2023

LarryOsterman commented May 24, 2023

antkmsft left a comment

antkmsft left a comment

Convert most AMQP types to read-only; Significant API surface cleanup. #4640

Convert most AMQP types to read-only; Significant API surface cleanup. #4640

Conversation

LarryOsterman commented May 17, 2023

Significant cleanup of AMQP internals.

Pull Request Checklist

LarryOsterman commented May 17, 2023

LarryOsterman commented May 22, 2023

LarryOsterman commented May 23, 2023

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft commented May 24, 2023

LarryOsterman commented May 24, 2023

antkmsft left a comment

Choose a reason for hiding this comment

antkmsft left a comment

Choose a reason for hiding this comment