@koxudaxi Thanks for the explanation; now it makes perfectly clear what the underlying issue was.
Originally posted by azatoth August 21, 2022
When I'm running the generator on https://ll.thespacedevs.com/2.2.0/schema (no option flags supplied to the binary), it will for the components Launcher
and LauncherConfig
, it will append a "1" to the class names.
I don't see any apparent conflicts in the spec, thus I'm a bit lost why this happens. It doesn't happens to any of the other components in the spec file.
Any input would be grateful.
I've boiled down the issue to following minimal spec:
openapi: 3.0.3
info:
title: Foo
version: "1.0"
paths:
/:
get:
responses:
'200':
description: ''
components:
schemas:
Foo:
type: object
properties:
foo_bar:
allOf:
- $ref: '#/components/schemas/FooBarOther'
FooBar:
type: object
properties:
id:
type: integer
FooBarOther:
type: object
properties:
id:
type: integer
That will result in following python code:
from __future__ import annotations
from typing import Optional
from pydantic import BaseModel
class FooBar1(BaseModel):
id: Optional[int] = None
class FooBarOther(BaseModel):
id: Optional[int] = None
class Foo(BaseModel):
foo_bar: Optional[FooBarOther] = None
I notice if I break out the $ref
from the allOf
, it will not generate a numbered class name.
Also that property name must be snake case of an existing model.
I'm positive this is not the intended result, thus I'm filing this issue.
@azatoth Thank you for creating this PR. I added some lines to fix the problem.
Thank you :) I didn't know what the underlying issue was, but I thought at least I could add a test to exemplify the issue.
Add test for allOf with common prefix and $ref
Per #714 test is needed to examplify the issue.
Per #714 test is needed to examplify the issue.
Allowing negative values is trivial and I can add it in core code. Just make the downloader handle +ive values, and make sure the extractor returns correct
timestamp
/release_timestamp
Can you explain what you mean by "+ive values"?
I must confess I've problem navigate the code; And I'm not sure what you want me to accomplish :/
Is there no way around this? If we can determine the current length of the stream (or the start time), we can translate b/w the two systems. This will also allow to implement #4554 and #5608 in future
While it would be possible to count from start of live stream, I think that would be impractical for most usecases as it would require the user to know exactly when the live stream started.
Counting from the MAX_DURATION
would make it practically impossible to use as the window would move.
I can think of two compromises:
--download-sections "@1671698944-1671698964"
--download-sections *-start--end
fix(apigateway): allow multi-level base path mapping (#23362)
API Gateway allows multi-level base path mapping but CDK doesn't support the same yet. (AWS Announcement Post)
Also, added new base path mapping validations such as shouldn't start or end with /
or shouldn't contain consecutive /
.
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
fix(autoscaling): Allow adding AutoScalingGroup to multiple target groups
While this doesn't solve the underlying problem, which will require an
API breakage in scaleOnRequestCount()
(see
https://github.com/aws/aws-cdk/issues/5667#issuecomment-636657482).
As scaleOnRequestCount()
seems to be the only part that is affected by
this, we'll only issue a validation error if we have multiple
targetGroupArns
and scaleOnRequestCount()
has been called.
closes #5667
Apply suggestions from code review
Co-authored-by: Calvin Combs 66279577+comcalvi@users.noreply.github.com
Properly name variable for checking if scale on request has been called
The naming and information didn't fully match eachother.
Split test for multiple target groups into two separate tests
this to clearly indicate that one is positive and the other is negative.
Options for begin and end of recording live streams
Extend --live-from-start
with two options -live-from-start-begin
and
-live-from-start-end
which will limit the recording to between these
values.
--live-from-start-end
is optional and if omitted will continously
record as normal.
Don't add new switches. Instead, make the feature obey
--download-sections *start-end
. The downloader can access the timestamps atsection_start/end
keys in the info_dict
So I can see --download-sections
to work if we add following restrictions when we are under a live stream:
imo making these stipulations makes a totally separate function than what we currently have.
it's explicitly prohibited to be used when live
This is because we don't have an implementation for it
I assume you have a totally different implementation in mind as I can't see it possible to utilize --download-sections
for this. neither to be able to specify sections (chapters), nor having multiple separate sections.
Don't add new switches. Instead, make the feature obey
--download-sections *start-end
. The downloader can access the timestamps atsection_start/end
keys in the info_dict
I was thinking about using that one but as it's explicitly prohibited to be used when live I thought it was better to not try to attempt that.
IMPORTANT: PRs without the template will be CLOSED
Options for begin and end of recording live streams
Extend --live-from-start
with two options -live-from-start-begin
and -live-from-start-end
which will limit the recording to between these values.
--live-from-start-end
is optional and if omitted will continously record as normal.
While as generic as the existing --live-from-start
option, it's limited to youtube for now.
Fixes #3451
Options for begin and end of recording live streams
Extend --live-from-start
with two options -live-from-start-begin
and
-live-from-start-end
which will limit the recording to between these
values.
--live-from-start-end
is optional and if omitted will continously
record as normal.
Option to define a range to grab for live streams on youtube
This looks really good! Could we add a unit test for the positive case too?
The test for the positive case was kinda in the test already; In any case, I did split the test into two separate tests in db1f0dee69e9587a147abf77154f9d50ba29dc3a to be more explicit.
Split test for multiple target groups into two separate tests
this to clearly indicate that one is positive and the other is negative.
Properly name variable for checking if scale on request has been called
The naming and information didn't fully match eachother.
feat(lambda): expose all docker run options to container bundling of all lambda variants (#23318)
This continues the work started in #22829 by exposing the underlying docker run options of the container bundling. With that the bundling feature can be used in a wider range of setups that the current defaults can not support out of the box.
The removed properties are covered in the same way by the one the interface extends from.
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
fix(s3-deployment): source markers missing when there are multiple sources
Follow up to #23321. There is an interesting edge case where if there is any source that has source markers then all sources have to have source markers. This is due to the way the custom resource logic works
https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L64
https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L137
updating integration tests
chore(release): 2.55.1
chore(release): 2.55.1 (#23368)
See CHANGELOG
fix(s3-deployment): source markers missing when there are multiple sources (#23364)
Follow up to #23321. There is an interesting edge case where if there is any source that has source markers then all sources have to have source markers. This is due to the way the custom resource logic works
https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L64
https://github.com/aws/aws-cdk/blob/02d0876bbb196e9fbeb32d977e7cf65229c8559d/packages/%40aws-cdk/aws-s3-deployment/lib/lambda/index.py#L137
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Merge branch 'main' into merge-back/2.55.1
chore(merge-back): 2.55.1 (#23369)
See CHANGELOG
feat(cfnspec): cloudformation spec v102.0.0 (#23372)
feat(integ-tests): add serializedJson on match utility (#23218)
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
feat(trigger): Allow trigger to work with Lambda functions with long timeouts (#23062)
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
chore(cli): make the typescript version compat task compatible with npm v6 (#23374)
Current script breaks with when npm v6 is installed due to the use of npm pkg
command which was introduced in npm v7. However npm v6 is still the default version for Node.js <=14.
yarn integ
to deploy the infrastructure and generate the snapshot (i.e. yarn integ
without --dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
fix(autoscaling): Allow adding AutoScalingGroup to multiple target groups
While this doesn't solve the underlying problem, which will require an
API breakage in scaleOnRequestCount()
(see
https://github.com/aws/aws-cdk/issues/5667#issuecomment-636657482).
As scaleOnRequestCount()
seems to be the only part that is affected by
this, we'll only issue a validation error if we have multiple
targetGroupArns
and scaleOnRequestCount()
has been called.
closes #5667
Apply suggestions from code review
Co-authored-by: Calvin Combs 66279577+comcalvi@users.noreply.github.com
Properly name variable for checking if scale on request has been called
The naming and information didn't fully match eachother.
Properly name variable for checking if scale on request has been called
The naming and information didn't fully match eachother.
Apply suggestions from code review
Co-authored-by: Calvin Combs 66279577+comcalvi@users.noreply.github.com