Zappa refactor#10
Conversation
|
This pull confuses me. With files coming and going, I can't figure out what's going on. Part of it may be my unfamiliarity with GitHub. I'm also unfamiliar with this codebase! Can you tell me more? If you're confident this is an improvement, I have no objections to merging. |
|
Replied to the wrong email, apologies again, my error I'll delete that.
The deploy-aws folder contains stand alone code that runs as an AWS
Lambda. The data converter was rewritten by me to (a) focus only on deals,
for now, and (b) to extend the output format of deals to not just PBN, but
also RBN, LIN and a "human readable" (Human) format.
…On Wed, 7 Oct 2020 at 20:41, Kiat Huang ***@***.***> wrote:
Apologies I should have explained. These files represent the proposed
converted end state of an RBN ascii file into JSON format, with examples.
They are not needed or suggested to be used programmatically. They can't
break anything.
On Fri, 2 Oct 2020 at 05:21, Adam Wildavsky ***@***.***>
wrote:
> This pull confuses me. With files coming and going, I can't figure out
> what's going on. Part of it may be my unfamiliarity with GitHub. I'm also
> unfamiliar with this codebase! Can you tell me more?
>
> If you're confident this is an improvement, I have no objections to
> merging.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#10 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEDQZW25NDXT2RHHWRJAOX3SIVINNANCNFSM4R5FRR5Q>
> .
>
--
Kind regards,
Kiat
linkedin.com/in/kiathuang <https://www.linkedin.com/in/kiathuang/>
bit.ly/bridge-hackathon
bswg.org
www.ebu.co.uk/biographies/kiat-huang
+44-7526-810553
--
Kind regards,
Kiat
linkedin.com/in/kiathuang <https://www.linkedin.com/in/kiathuang/>
bit.ly/bridge-hackathon
bswg.org
www.ebu.co.uk/biographies/kiat-huang
+44-7526-810553
|
Sure, this folder and code are new and does not replace the original files in the main dir which stand unchanged.
This code is intended to run as a Cloud function and indeed works. It can be tested with the following command or similar. Input from the shell Output |
|
Sounds good. I'll try to have a look at dealConverter.py by this weekend.
Are there now two versions of the code? I hate to see duplication - I don't
know whether it's avoidable here.
FYI, my preferences for code review differ from some others. I prefer to
review as small chunks as possible. One-line changes, or one-character
changes, are fair game!
Do you have unit tests for dealConverter.py? I think you should. The
benefits are many - one is that it makes code review easier.
…On Tue, Oct 13, 2020 at 11:20 AM Kiat ***@***.***> wrote:
This pull confuses me. With files coming and going, I can't figure out
what's going on. Part of it may be my unfamiliarity with GitHub. I'm also
unfamiliar with this codebase! Can you tell me more?
Sure, this folder and code are new and does not replace the original files
in the main dir which stand unchanged.
-
dealConverter.py (app) - rewrite, just converts deals encoded in UUID
to any of PBN, RBN, LIN, human, formats.
I used the algorithm described by Richard Pavlicek and converted it to
code.
-
zappa_settings.json (config) - a config for Zappa which is an
orchestration tool that works with AWS
-
requirements.txt (pip3 install -r )
-
README.md
This code is intended to run as a Cloud function and indeed works. It can
be tested with the following command or similar.
*Input from the shell*
curl --header "Content-Type: application/json" --request POST --data
'{"output":"human","uuid":"0x63b40b7f2842298cb3bea48"}'
https://converter.aws.globalbridge.app/api
*Output*
{"body":
{"E":"Q982.KQ96.87.984","N":"AKT65.AT3.KT6.62","S":"3.85.AQJ432.AT53","W":"J74.J742.95.KQJ7"},"statusCode":200}
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABC4PYGUDZZPDI6GC7IDLIDSKRV3HANCNFSM4R5FRR5Q>
.
|
|
It's hard to do an incremental "few lines of code" change when one creates
a new set of files!
As mentioned this is a "port" of the current Deal converter, which takes a
PBN file as input (i.e. Results) and returns JSON. And chose deployment as
a Cloud Function, rather than as a service on our own servers (GCP K8S)
I wanted to have a pure Deal converter that takes any recognized Deal
format and returns any other requested. A use case is Deal uses this
microservice to convert it's fresh UUID deals in the format the user has
requested, e.g. PBN.
I've started with UUID in and PBN, RBN, LIN, Human out.
Good point about the unit tests, I haven't done them and will write them
and add to the PR.
…On Tue, 13 Oct 2020, 21:36 Adam Wildavsky, ***@***.***> wrote:
Sounds good. I'll try to have a look at dealConverter.py by this weekend.
Are there now two versions of the code? I hate to see duplication - I don't
know whether it's avoidable here.
FYI, my preferences for code review differ from some others. I prefer to
review as small chunks as possible. One-line changes, or one-character
changes, are fair game!
Do you have unit tests for dealConverter.py? I think you should. The
benefits are many - one is that it makes code review easier.
On Tue, Oct 13, 2020 at 11:20 AM Kiat ***@***.***> wrote:
> This pull confuses me. With files coming and going, I can't figure out
> what's going on. Part of it may be my unfamiliarity with GitHub. I'm also
> unfamiliar with this codebase! Can you tell me more?
>
> Sure, this folder and code are new and does not replace the original
files
> in the main dir which stand unchanged.
>
> -
>
> dealConverter.py (app) - rewrite, just converts deals encoded in UUID
> to any of PBN, RBN, LIN, human, formats.
> I used the algorithm described by Richard Pavlicek and converted it to
> code.
> -
>
> zappa_settings.json (config) - a config for Zappa which is an
> orchestration tool that works with AWS
> -
>
> requirements.txt (pip3 install -r )
> -
>
> README.md
>
> This code is intended to run as a Cloud function and indeed works. It can
> be tested with the following command or similar.
>
> *Input from the shell*
> curl --header "Content-Type: application/json" --request POST --data
> '{"output":"human","uuid":"0x63b40b7f2842298cb3bea48"}'
> https://converter.aws.globalbridge.app/api
>
> *Output*
>
> {"body":
>
{"E":"Q982.KQ96.87.984","N":"AKT65.AT3.KT6.62","S":"3.85.AQJ432.AT53","W":"J74.J742.95.KQJ7"},"statusCode":200}
>
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <
#10 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ABC4PYGUDZZPDI6GC7IDLIDSKRV3HANCNFSM4R5FRR5Q
>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEDQZW4JUN2WAZXTZ4VHWVTSKS25LANCNFSM4R5FRR5Q>
.
|
Rewrote the converter to handle UUID deal conversion to PBN, RBN, LIN, and Human (readable) formats. Refactored the two files (local py and api py) into one file, with Flask integration and deployment by Zappa.
Simplified the number of files but my code does not handle the upload of a PBN file, just parsing UUID, so it's not a direct replacement of the original deployment. I'd like to add the reverse conversion from these formats to UUID, so that with at most two API calls, conversion can occur between any pair of formats via UUID.