search

loic-sharma / BaGet

A lightweight NuGet and symbol server
https://loic-sharma.github.io/BaGet/
MIT License
2.62k stars 674 forks source link

Separate server controllers into a dedicated project and publish as package #180

Open nimrodolev opened 5 years ago

nimrodolev commented 5 years ago

Feature

Separate the BaGet controllers (and other associated code, like HttpRequestExtensions) into a dedicated project that will be built into a package and published.

Why?

It would be awesome if the BaGet controllers could be consumed as an independent package. This could allow one to compose his/her own server implementation without making changes to the main BaGet project, while still benefiting from any additions, improvements or bug fixed that will be made to this area of the code in the future. Examples for things this change will allow -

  1. Creating custom default behaviors.
  2. Easily integrating a different UI, or just dropping it all together.
  3. Supporting other configuration methods/providers/conventions.
  4. Cancelling the inclusion of auxiliary packages (e.g. AWS, Azure, EF) if they are not used.
  5. Change hosting solutions easily (which is actually where I'm coming from).

My use case

I'm thinking of creating a serverless BaGet server over AWS. The idea is to have the server hosted as a Lambda function and exposed through AWS ApiGateway. Storage will be S3 based (which I saw is pretty much supported already), and I'll create a PackageService and a SearchService that are based on AWS SimpleDB.

At the end you have a private BaGet server in the cloud that costs nothing except for the S3 storage for the packages (so, virtually nothing).

Cons

Pros

loic-sharma commented 5 years ago

I like this idea! As an added bonus, this will trim down the BaGet project to just be a minimal wrapper around BaGet devices.

nimrodolev commented 5 years ago

:smiley_cat: I'd be happy to make the changes and create a PR for this, would that work?

loic-sharma commented 5 years ago

That’d be great. I’m currently out for the holidays l, so I won’t be able to review your PR until next week

nimrodolev commented 5 years ago

No problem - I actually forked the repo and started fiddling around with the code yesterday. Moving the code to a separate project and using it with a reference was pretty simple and quick, but now some of the tests got broken (something with the SPA middleware isn't playing nicely). Anyway, hopefully I'll have it sorted out by the time your'e back.

Happy holidays :fireworks:

WernerMairl commented 5 years ago

hi nimrodolev

i like the idea... can i help you in some way ?

lessons learned in the past: smaller PRs are easier to handle.

my suggest: the first PR about this should contain only the (new) csproj and the reference changes/addings between the old and new csproj... but NO bigger code (*.cs) moves.

After this it is easier for other people to help with the next (small) steps... moving classes etc....

regards Werner

nimrodolev commented 5 years ago

Hi @WernerMairl,

Thanks for help!

I actually have the all of the code changes done (they were not pushed to my fork up until now). I can revert things a bit an send out a PR with just the empty new project, if that's better - but seeing as there is no code change (just file movements) I'm not sure that's actually needed.

As for help - what I'm currently stuck over (I didn't have a lot of time to dig into this yet) is that some of the tests were broken after I made the change. This seems to be related to the SPA middleware not finding the SPA files, something with the hard-coded file paths (in the Startup.cs).

I pushed all of the current changes to my fork (here) - I'd appreciate it if you take a look and let me know if you think I should try to fix the broken tests or just revert things to a simpler state with just the new, empty, project and send out the PR a "simple" PR.

Thanks, Nimrod

WernerMairl commented 5 years ago

the following hunk should solve your problem with the not working tests: image

nimrodolev commented 5 years ago

Thanks Werner, that totally worked! :+1: I pushed the change with the working tests, merged a few commits that were done to BaGet after I forked, and issued a PR. This contains all of the changes, not just the empty project. If you or @loic-sharma find this too big let me know and I can close it and issue a new one with just the empty project, and we can move on from there.

Thanks for the help! Nimrod

WernerMairl commented 5 years ago

Idea

To validate some idea around "BaGet modularization" i have created the following draft:

Environment

BaGet Instance running on Azure

Running on this Url: https://mybaget.azurewebsites.net/ Deployed from this Repo/Fork: https://github.com/WernerMairl/CustomBaGet/tree/feature/init/src

Approach

The repo does NOT contains BaGet source code but a minimalstic dotnet core csproj.

In this repo i have created a folder "References" that contains the compiler output from the PR created by Nimrod above plus a folder "BaGet.UI/build" that contains the (builded!!) frontend.

The assemblies in folder "References" are added to the csproj by using Assembly references (later NuGet-References)

the files in "BaGet.UI/build" are added by using a msbuild targe inside csproj (stolen and adopted from the original BaGet project) INCLUDING BaGet.dll /the main project currently not available as nupkg)

and... it works (hosted on Azure)

I have found a few issues/questions/new requirements for BaGet during this try. They are noted as issues in the repo https://github.com/WernerMairl/CustomBaGet

Waiting for a discussion with @loic-sharma (after holidays) if it is worth to follow the idea (and create some issue/pr here in the main project)

nimrodolev commented 5 years ago

@WernerMairl, that's super cool :smile:

loic-sharma commented 5 years ago

That's looking awesome!