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

Build a reliable release life cycle #213

Closed
pfreixes opened this issue May 17, 2017 · 11 comments
Closed

Build a reliable release life cycle #213

pfreixes opened this issue May 17, 2017 · 11 comments
Labels

Comments

@pfreixes
Copy link

Hi @jettify sorry the channel used to approx you, I didn't know how to do it.

Right now the unique serious attempt to have botocore working in an asynchronous environment is this project, unfortunately IMHO there is still work to do to give a full asynchronous package working for all of the AWS services. I would like to help in the direction to have this package in the best position to make it an alternative to the official synchronous SDK, how?

  1. Functional tests

Botocore has a set of end to end tests that check different execution paths from the client function to a mocked version of the HTTP response. This ensures that all of the glue between the client and the network layer works properly.

I will propose to "copy" all functional tests from the botcore repository using aiobotcore, this will help this package in three different things:

  • Increase the coverage making sure that the aiobotcore substitution does not break the glue and the end to end tests work properly.
  • Raise unnoticed bugs
  • Build a methodology that allows aiobotcore to relay in something at each new botcore release
  1. Integration tests

The same as the functional ones, but in this case having the issue to the need of have a real AWS account. Here, most of the tests end up hitting real AWS resources. Here I can try to involve my organization to allocate this resources. However, I would like to know how the people from botcore is doing this process in their releases, taking into account that these tests are not part of the public CI /cc @jamesls

This also comes with the same benefits already commented in the previous point.

  1. Version alignment

Why not go for an alignment of the versions between botcore and aiobotcore ? This will help at least in the following things:

  • A cycle of releases based on publishes a new aiobotcore when a new botcore version appears. That would imply among other things update the functional and integration tests, relaying on the tests results to make finally the aligned release.
  • Make the life easier for the developers.
  1. Move the current test suite to something like ut

... and remove those ones that are in fact a duplication of the integrations/functional that come from botocore

IMHO these four things might help this project to get more velocity, and the most important have a solid foundation to make the release life cycle more trustable. I know that this is work, and in some way can be considered a tedious work. Regarding this, I could try to organize a hackathon to make it possible.

Let me know what do you think ...

@jettify
Copy link
Member

jettify commented May 18, 2017

Hi @pfreixes I think it is good place to have this conversation. I and @thehesiod maintain this package and we appreciate any help we can get and always open to suggestions.

botocore is largish (20k source , 20k tests) project we try hard to reuse/import code from there. As result we ported most of s3 functional tests, that covers s3 API since we both use it somewhere in other projects. For other services we have simple tests just to see it they work at all (most of them contributed by users)

We have some points partially addressed

  1. 2 We use moto mocking server as aws substitution. It is started as separate process and aiobotocore hits it over HTTP. Real aws would be better, but this what we have now.

  2. We have bot that creates PR on each botocore update (like this one Update botocore to 1.5.51 #214) so if PR is green with new botocore we merge it, then manually bump possible version in setup.py on next aiobotocore version. I have to admit we do releases less frequently. If there is need for this we can align with botocore releases.

To summarize

  1. we will be very gratefully for any help with testing, we can create plan with list of tests that need to be ported, and create related issues, so community can help. Not sure how to address aws keys issue so mocking server is way to go for now.
  2. align releases, something we can do today

@thehesiod what is your opinion?

@thehesiod
Copy link
Collaborator

thehesiod commented May 18, 2017

btw big caveat with #2, is that even if it's green, we need to manually check the changes to ensure they didn't touch sections of files we're replacing.

ya ideally we could re-use the botocore tests.

I was just thinking maybe a way of doing this would be to have an adapter botocore module which does loop.run_until_complete against aiobotocore for each botocore API. If it uses multiple threads we can use a runloop per thread.

@pfreixes
Copy link
Author

Yea that's the "problem" of this methodology, but I'm still pretty convinced that this is a small problem if you compare with the current strategy.

Getting the diff between the last change in the major version, from 1.4.49 to 1.5.0 there were only a few changes in one integration test tests/integration/test_client.py. Also getting some random diffs between the minor releases the changes are minimum about functional and integration tests.

The issues with moto, something that can be useful for environments that have to reproduce a spefic test cases, is that you must reproduce all scenarios and all of the casuistics before saying that your library covers the functionality expected by any user that is keen on use an asynchronous version of the botocore library.

Here the question IMHO is, the integration tests and the functional tests can ensure that? My guess is yes. I asked some guy from botocore to land here, to get a response from someone that should have more concrete information and probably the response.

About the use use of an adapter I don't get the point, correct me if I'm wrong, but the current implementation of aiobotocore its based on override those necessary functions that are in contact with the adapter thing (urllib3) to plug there aiohttp iplementing then the same contract for the upper layers via proxies or whatever. Are you thinking in change this pattern?

@thehesiod
Copy link
Collaborator

btw new update is I've just pushed does hash checks for the methods we replace/override to help us do automatic version bumps and point us to areas that need to be investigated (as a temporary helper).

I think what @jettify means is that if there were an official way to replace botocore's underlying HTTP stack we'd then it would greatly reduce the complexity of integration. Further if done correctly would theoretically allow re-use of botocore's unittests. Lets keep up the convo! On a related note there's an AWS c++ API: https://github.com/aws/aws-sdk-cpp. I think ideally the AWS c++ API would support async, and the final botocore would use the aws c++ library underneath and provide sync/async APIs.

@thehesiod
Copy link
Collaborator

FYI we have this related issue: #36

@pfreixes
Copy link
Author

Yeps @thehesiod ..

I didn't know or I wasn't able to find the #36 when I've opened this issue. In any case, I can understand your point of view and the rationale exposed in some of the comments in #36 that is definitely aligned with the main idea of keeping just a core and inject the async implementation in somehow. That, in an ideal world, will give us the chance to make de facto aiobotocore full compatible with botocore.

At first impression, my first concern with that way is known if there is some piece of the code that does not allow to do so. I'm not talking about the class factory or whatever that has to be injected to allow that mutation, indeed this is something that can be addressed easily with a PR. My concern here, is if there are some parts of the code inside of botocore that are coupled to the IO. Perhaps, doing some composition making many HTTP calls, something similar of boto3. If it happens, this approx might not be achievable, being forced to rewrite these kind of functions that will require an extra effort and increase the difficulty to have something clean, automatic and tested . Are you aware of that? exists this red flag ?

If this way becomes achievable I don't have any doubt that this is the clean and less time-consuming way to make it happen, rather than the proposal that I made.

Regarding the C++ library, I've just taken a quick glance to them but to close all of the knots to have a underlying library that might support async/sync environments is IMHO still far away with the current status of that library. For example, I couldn't see any place that will allow you to register callbacks to bind the events with your loop. Looks like the under the hood they are using Curl and a thread safe connection pool.

BTW requests/urllib3 is discussing in change the implementation from sync to async. But, giving support for both worlds. As you know botocore uses urllib3, therefore at very long term we could see an automatic support for the async world because of that. But as I said just discussions and public statements, but nothing concrete in terms of code.

@thehesiod
Copy link
Collaborator

thehesiod commented Jun 22, 2017

@pfreixes great discussions! ya, I discussed with the c++ devs (aws/aws-sdk-cpp#122) about my idea and they said they were thinking about it but ya everything is still far away :) Feel free to open issues to help us move in a direction where aiobotocore keeps on shrinking. So the only issue that really comes to mind are things like core http calls: #2. Other things like async for for paginators can be supported by the paginator class dynamically supporting the type of iteration based on your "mode."

@pfreixes
Copy link
Author

what can I see here #2 its just a matter of fix the PR to make it compatible with the last changes in botocore, nothing a big deal? or I'm missing something?

Ok let's guess that we can address all of these issues, let's try to answer what IMHO is the most important question:

How to check that a new botocore release is compatible with the current master of aiobotocore allowing to release automatically a new aiobotocore paired with the botocore one?

My concern about that is. Despite, we will be able to inject the perfect layer to make botocore async we will be still forced to check the whole functionality. And even in that case, the integration/acceptance/functional tests that are provided by botocore must be modified to make them asynchronous. And this is still a huge and time-consuming work, having also the disadvantage to keep them synchronized with the original ones. And this is for me the same problem that you raised.

Here I can see only three solutions:

  1. Rewrite the test suite to make it compatible with the asynchronous world. The disadvantages are clear enough.
  2. Introduce a thin layer between the test suite and aiobotocore as an extra step for the release to check the test suite to check it with the asynchronous client. No fucking idea how to do it right now, but will reduce a lot the effort needed.
  3. The rationale of run the test suite is invalid.

PD : IMHO when I'm referring to the test suite, I would like to put the focus on that tests that cover end 2 end functionality as a way to check that the new intermediate layer does break nothing. The unittest IMHO are out of that scope.

@thehesiod thehesiod changed the title Build a release life cycle more reliable Build a reliable release life cycle Jul 3, 2017
@thehesiod
Copy link
Collaborator

so I was thinking about this again and I think we could probably get all the tests working with an adapter layer, however it's going to be really complicating due to mocking, where botocore can replace individual methods/properties on classes, and this wouldn't make sense in aiobotocore since it would most likely be an async method...which means it would need to have an adapter as well. Getting a test adapter is going to require a lot of work and would definitely require a hackathon

@thehesiod
Copy link
Collaborator

plus, we're not stable yet, as we have a BIG upcoming change to correctly support the credentials class and a few other tidbits. We'd need to be feature complete before we even started this endeavor.

Copy link

This issue has been marked as stale because it has been inactive for more than 60 days. Please update this pull request or it will be automatically closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 20, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants